Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1229968 > unrolled thread

Re: [PATCH 1/2] nios2: Add architectural support for PCIe

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/2] nios2: Add architectural support for PCIe Arnd Bergmann <arnd@arndb.de> - 2015-09-22 10:20 +0200
    Re: [PATCH 1/2] nios2: Add architectural support for PCIe Ley Foon Tan <lftan@altera.com> - 2015-09-22 13:00 +0200
      Re: [PATCH 1/2] nios2: Add architectural support for PCIe Arnd Bergmann <arnd@arndb.de> - 2015-09-22 13:50 +0200
        Re: [PATCH 1/2] nios2: Add architectural support for PCIe Ley Foon Tan <lftan@altera.com> - 2015-09-29 05:20 +0200
          Re: [PATCH 1/2] nios2: Add architectural support for PCIe Arnd Bergmann <arnd@arndb.de> - 2015-09-29 09:40 +0200
            Re: [PATCH 1/2] nios2: Add architectural support for PCIe Ley Foon Tan <lftan@altera.com> - 2015-09-29 12:10 +0200

#1229968 — Re: [PATCH 1/2] nios2: Add architectural support for PCIe

On Tuesday 22 September 2015 15:19:26 Ley Foon Tan wrote:
> +
> +config PCI_SYSCALL
> +	def_bool PCI

IIRC, PCI_SYSCALL is deprecated and you should just leave that
turned off.

In any case, you enable the syscalls here but don't assign a
system call number, so that is rather pointless. I 

I might be missing something though. Bjorn?

> @@ -14,7 +14,7 @@
>  #include <linux/types.h>
>  #include <asm/pgtable-bits.h>
>  
> -/* PCI is not supported in nios2, set this to 0. */
> +/* PCI I/O space is not supported, set this to 0. */
>  #define IO_SPACE_LIMIT 0
>  
>  #define readb_relaxed(addr)	readb(addr)

It might be useful to enable this, just in case someone connects
a PCI host bridge that does support I/O space.

Which host bridge do you use? Are you sure there is no I/O space?
Most of them use a set of translation windows to set up a mapping
between bus address (memory, config and io space) and physical
(mmio) space.

> diff --git a/arch/nios2/include/asm/pci.h b/arch/nios2/include/asm/pci.h
> new file mode 100644
> index 0000000..f2cba05
> --- /dev/null
> +++ b/arch/nios2/include/asm/pci.h

What happens if you use the asm-generic header? If there is something
missing in it, we can try to get it to do the right things.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1230076

On Tue, Sep 22, 2015 at 4:18 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 22 September 2015 15:19:26 Ley Foon Tan wrote:
>> +
>> +config PCI_SYSCALL
>> +     def_bool PCI
>
> IIRC, PCI_SYSCALL is deprecated and you should just leave that
> turned off.
>
> In any case, you enable the syscalls here but don't assign a
> system call number, so that is rather pointless. I
You are right. I can remove it if it is deprecated .
>
> I might be missing something though. Bjorn?
>
>> @@ -14,7 +14,7 @@
>>  #include <linux/types.h>
>>  #include <asm/pgtable-bits.h>
>>
>> -/* PCI is not supported in nios2, set this to 0. */
>> +/* PCI I/O space is not supported, set this to 0. */
>>  #define IO_SPACE_LIMIT 0
>>
>>  #define readb_relaxed(addr)  readb(addr)
>
> It might be useful to enable this, just in case someone connects
> a PCI host bridge that does support I/O space.
>
> Which host bridge do you use? Are you sure there is no I/O space?
> Most of them use a set of translation windows to set up a mapping
> between bus address (memory, config and io space) and physical
> (mmio) space.
I'm using Altera PCIe IP. But, you are right. We should enable this,
because it shouldn't limited to support Altera PCIe only.
BTW, any rule/requirement for the IO_SPACE_LIMIT?
And PCIBIOS_MIN_IO should be non-zero if we enable IO_SPACE_LIMIT?

>
>> diff --git a/arch/nios2/include/asm/pci.h b/arch/nios2/include/asm/pci.h
>> new file mode 100644
>> index 0000000..f2cba05
>> --- /dev/null
>> +++ b/arch/nios2/include/asm/pci.h
>
> What happens if you use the asm-generic header? If there is something
> missing in it, we can try to get it to do the right things.
All the defines in arch/nios2/include/asm/pci.h are required by PCI framework.
It will trigger undefined symbols/define error if without them. Seem
all architectures must provide all these defines.
So, we can consider to add default defines for these in asm-generic.

Regards
Ley Foon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230099

On Tuesday 22 September 2015 18:53:50 Ley Foon Tan wrote:
> On Tue, Sep 22, 2015 at 4:18 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 22 September 2015 15:19:26 Ley Foon Tan wrote:
> > I might be missing something though. Bjorn?
> >
> >> @@ -14,7 +14,7 @@
> >>  #include <linux/types.h>
> >>  #include <asm/pgtable-bits.h>
> >>
> >> -/* PCI is not supported in nios2, set this to 0. */
> >> +/* PCI I/O space is not supported, set this to 0. */
> >>  #define IO_SPACE_LIMIT 0
> >>
> >>  #define readb_relaxed(addr)  readb(addr)
> >
> > It might be useful to enable this, just in case someone connects
> > a PCI host bridge that does support I/O space.
> >
> > Which host bridge do you use? Are you sure there is no I/O space?
> > Most of them use a set of translation windows to set up a mapping
> > between bus address (memory, config and io space) and physical
> > (mmio) space.
>
> I'm using Altera PCIe IP. But, you are right. We should enable this,
> because it shouldn't limited to support Altera PCIe only.
> BTW, any rule/requirement for the IO_SPACE_LIMIT?

If you want to allow just one PCI host bridge, set it to 0xffff, otherwise
use 0xfffff (for up to 16 bridges)

> And PCIBIOS_MIN_IO should be non-zero if we enable IO_SPACE_LIMIT?

If you define pcibios_align_resource() as an empty function, that can
remain zero as well, it's only meant to stay out of the way of ISA
devices.

> >> diff --git a/arch/nios2/include/asm/pci.h b/arch/nios2/include/asm/pci.h
> >> new file mode 100644
> >> index 0000000..f2cba05
> >> --- /dev/null
> >> +++ b/arch/nios2/include/asm/pci.h
> >
> > What happens if you use the asm-generic header? If there is something
> > missing in it, we can try to get it to do the right things.
> All the defines in arch/nios2/include/asm/pci.h are required by PCI framework.
> It will trigger undefined symbols/define error if without them. Seem
> all architectures must provide all these defines.
> So, we can consider to add default defines for these in asm-generic.

I think that would be good, yes. We probably picked defaults for the
asm-generic version that are enough to build with CONFIG_PCI disabled,
but there are useful defaults for the other case as well.

Most architectures that use the file don't support PCI, so you can just
add stuff inside of #ifdef CONFIG_PCI. For the remaining 13 architectures
(git grep -l generic/pci.h  | wc), see whether there are conflicts first.
The normal way would be to have the arch specific definition come before
the #include, and then do something like

#ifndef pci_proc_domain
#define pci_proc_domain pci_proc_domain
static inline int pci_proc_domain(struct pci_bus *bus)
{
#ifdef CONFIG_PCI_DOMAINS_GENERIC
        return pci_domain_nr(bus);
#else
	return 1;
}
#endif

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234668

On Tue, Sep 22, 2015 at 7:42 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 22 September 2015 18:53:50 Ley Foon Tan wrote:
>> On Tue, Sep 22, 2015 at 4:18 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Tuesday 22 September 2015 15:19:26 Ley Foon Tan wrote:
>> > I might be missing something though. Bjorn?
>> >
>> >> @@ -14,7 +14,7 @@
>> >>  #include <linux/types.h>
>> >>  #include <asm/pgtable-bits.h>
>> >>
>> >> -/* PCI is not supported in nios2, set this to 0. */
>> >> +/* PCI I/O space is not supported, set this to 0. */
>> >>  #define IO_SPACE_LIMIT 0
>> >>
>> >>  #define readb_relaxed(addr)  readb(addr)
>> >
>> > It might be useful to enable this, just in case someone connects
>> > a PCI host bridge that does support I/O space.
>> >
>> > Which host bridge do you use? Are you sure there is no I/O space?
>> > Most of them use a set of translation windows to set up a mapping
>> > between bus address (memory, config and io space) and physical
>> > (mmio) space.
>>
>> I'm using Altera PCIe IP. But, you are right. We should enable this,
>> because it shouldn't limited to support Altera PCIe only.
>> BTW, any rule/requirement for the IO_SPACE_LIMIT?
>
> If you want to allow just one PCI host bridge, set it to 0xffff, otherwise
> use 0xfffff (for up to 16 bridges)
Okay, will set it to 0xffff.
>
>> And PCIBIOS_MIN_IO should be non-zero if we enable IO_SPACE_LIMIT?
>
> If you define pcibios_align_resource() as an empty function, that can
> remain zero as well, it's only meant to stay out of the way of ISA
> devices.
Okay.
>
>> >> diff --git a/arch/nios2/include/asm/pci.h b/arch/nios2/include/asm/pci.h
>> >> new file mode 100644
>> >> index 0000000..f2cba05
>> >> --- /dev/null
>> >> +++ b/arch/nios2/include/asm/pci.h
>> >
>> > What happens if you use the asm-generic header? If there is something
>> > missing in it, we can try to get it to do the right things.
>> All the defines in arch/nios2/include/asm/pci.h are required by PCI framework.
>> It will trigger undefined symbols/define error if without them. Seem
>> all architectures must provide all these defines.
>> So, we can consider to add default defines for these in asm-generic.
>
> I think that would be good, yes. We probably picked defaults for the
> asm-generic version that are enough to build with CONFIG_PCI disabled,
> but there are useful defaults for the other case as well.
>
> Most architectures that use the file don't support PCI, so you can just
> add stuff inside of #ifdef CONFIG_PCI. For the remaining 13 architectures
> (git grep -l generic/pci.h  | wc), see whether there are conflicts first.
> The normal way would be to have the arch specific definition come before
> the #include, and then do something like
>
> #ifndef pci_proc_domain
> #define pci_proc_domain pci_proc_domain
> static inline int pci_proc_domain(struct pci_bus *bus)
> {
> #ifdef CONFIG_PCI_DOMAINS_GENERIC
>         return pci_domain_nr(bus);
> #else
>         return 1;
> }
> #endif

Okay, will try add them to asm-generic.
Do you prefer to send it as separate patch or can be part of this patchset?
Thanks.

Regards
Ley Foon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234776

On Tuesday 29 September 2015 11:12:24 Ley Foon Tan wrote:
> >
> > I think that would be good, yes. We probably picked defaults for the
> > asm-generic version that are enough to build with CONFIG_PCI disabled,
> > but there are useful defaults for the other case as well.
> >
> > Most architectures that use the file don't support PCI, so you can just
> > add stuff inside of #ifdef CONFIG_PCI. For the remaining 13 architectures
> > (git grep -l generic/pci.h  | wc), see whether there are conflicts first.
> > The normal way would be to have the arch specific definition come before
> > the #include, and then do something like
> >
> > #ifndef pci_proc_domain
> > #define pci_proc_domain pci_proc_domain
> > static inline int pci_proc_domain(struct pci_bus *bus)
> > {
> > #ifdef CONFIG_PCI_DOMAINS_GENERIC
> >         return pci_domain_nr(bus);
> > #else
> >         return 1;
> > }
> > #endif
> 
> Okay, will try add them to asm-generic.

Thanks

> Do you prefer to send it as separate patch or can be part of this patchset?

I prefer the patches to be part of this series and merged through together
with them.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234937

On Tue, Sep 29, 2015 at 3:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 29 September 2015 11:12:24 Ley Foon Tan wrote:
>> >
>> > I think that would be good, yes. We probably picked defaults for the
>> > asm-generic version that are enough to build with CONFIG_PCI disabled,
>> > but there are useful defaults for the other case as well.
>> >
>> > Most architectures that use the file don't support PCI, so you can just
>> > add stuff inside of #ifdef CONFIG_PCI. For the remaining 13 architectures
>> > (git grep -l generic/pci.h  | wc), see whether there are conflicts first.
>> > The normal way would be to have the arch specific definition come before
>> > the #include, and then do something like
>> >
>> > #ifndef pci_proc_domain
>> > #define pci_proc_domain pci_proc_domain
>> > static inline int pci_proc_domain(struct pci_bus *bus)
>> > {
>> > #ifdef CONFIG_PCI_DOMAINS_GENERIC
>> >         return pci_domain_nr(bus);
>> > #else
>> >         return 1;
>> > }
>> > #endif
>>
>> Okay, will try add them to asm-generic.
>
> Thanks
>
>> Do you prefer to send it as separate patch or can be part of this patchset?
>
> I prefer the patches to be part of this series and merged through together
> with them.
>
Okay, will send a new version later.

Thanks.

Regards
Ley Foon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web