Sharing PCIE driver between Microblaze and Arm zynq

Sharing PCIE driver between Microblaze and Arm zynq

[Michal Simek]

Hi guys,

I have a question regarding to sharing generic OF pcie driver between
two architectures MB and ARM Zynq.
Is drivers/pci/pcie location good for it?
Make no sense to have the same driver in two locations.
Is using readl/writel IO functions in this driver the best option
which we can have?
Or is there any other recommendation?

Also just want to check if it is correct to use pcie device_type.
The rest should be the same with pci description:
(http://devicetree.org/MPC5200:PCI#PCI_Address_Translation).

Thanks,
Michal

Re: Sharing PCIE driver between Microblaze and Arm zynq

[Bjorn Helgaas]

[+cc linux-pci]

On Thu, Dec 6, 2012 at 7:23 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> Hi guys,
>
> I have a question regarding to sharing generic OF pcie driver between
> two architectures MB and ARM Zynq.
> Is drivers/pci/pcie location good for it?
> Make no sense to have the same driver in two locations.

I think you're talking about a PCI host bridge driver.  It would
definitely be nice to move toward a generic, shared driver.  Host
bridge drivers are responsible for enumerating the PCI hierarchy below
the bridge.  Enumeration is not really PCIe-specific, so I wouldn't
put it in drivers/pci/pcie.

> Is using readl/writel IO functions in this driver the best option
> which we can have?
> Or is there any other recommendation?

I'm not really a driver person, but if you're writing a new driver,
wouldn't you use the iomap interfaces (ioremap(), ioread32(), etc)
rather than readl()?

> Also just want to check if it is correct to use pcie device_type.

I don't know what you're asking here.  Can you elaborate or give a
specific example?

> The rest should be the same with pci description:
> (http://devicetree.org/MPC5200:PCI#PCI_Address_Translation).

Re: Sharing PCIE driver between Microblaze and Arm zynq

[Michal Simek]

On 12/06/2012 10:27 PM, Bjorn Helgaas wrote:
> [+cc linux-pci]
>
> On Thu, Dec 6, 2012 at 7:23 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> Hi guys,
>>
>> I have a question regarding to sharing generic OF pcie driver between
>> two architectures MB and ARM Zynq.
>> Is drivers/pci/pcie location good for it?
>> Make no sense to have the same driver in two locations.
>
> I think you're talking about a PCI host bridge driver.  It would
> definitely be nice to move toward a generic, shared driver.  Host
> bridge drivers are responsible for enumerating the PCI hierarchy below
> the bridge.  Enumeration is not really PCIe-specific, so I wouldn't
> put it in drivers/pci/pcie.

Not a PCI expert, just trying to find out the proper location for this shared driver.


>> Is using readl/writel IO functions in this driver the best option
>> which we can have?
>> Or is there any other recommendation?
>
> I'm not really a driver person, but if you're writing a new driver,
> wouldn't you use the iomap interfaces (ioremap(), ioread32(), etc)
> rather than readl()?

That driver exists but it is not in mainline and it is better to directly
add it to proper location with correct io functions.
The question is if ioread/iowrite functions are correct one.
PowerPC io-defs.h suggests that readl/writel should be used for PCI.


>> Also just want to check if it is correct to use pcie device_type.
>
> I don't know what you're asking here.  Can you elaborate or give a
> specific example?

If node property device_type pcie should be used.
device_type = "pcie";

For pci it is device_type = "pci";
For network it is device_type = "network"
the same for serial, watchdogs.

Do we have any list of device_types which we should use?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: Sharing PCIE driver between Microblaze and Arm zynq

[Grant Likely]

On Thu, 6 Dec 2012 15:23:14 +0100, Michal Simek <michal.simek@xilinx.com> wrote:
> Hi guys,
> 
> I have a question regarding to sharing generic OF pcie driver between
> two architectures MB and ARM Zynq.
> Is drivers/pci/pcie location good for it?
> Make no sense to have the same driver in two locations.
> Is using readl/writel IO functions in this driver the best option
> which we can have?
> Or is there any other recommendation?
> 
> Also just want to check if it is correct to use pcie device_type.
> The rest should be the same with pci description:
> (http://devicetree.org/MPC5200:PCI#PCI_Address_Translation).

Yes. device_type may be deprecated, but the PCI binding predates that.
PowerPC uses pcie in the device type. arch/powerpc/kernel/of_platform.c

Some of that powerpc DT PCI support code really should be made generic.

g.

Re: Sharing PCIE driver between Microblaze and Arm zynq

[Grant Likely]

On Fri, 07 Dec 2012 10:45:01 +0100, Michal Simek <monstr@monstr.eu> wrote:
> On 12/06/2012 10:27 PM, Bjorn Helgaas wrote:
> > [+cc linux-pci]
> >
> > On Thu, Dec 6, 2012 at 7:23 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> >> Hi guys,
> >>
> >> I have a question regarding to sharing generic OF pcie driver between
> >> two architectures MB and ARM Zynq.
> >> Is drivers/pci/pcie location good for it?
> >> Make no sense to have the same driver in two locations.
> >
> > I think you're talking about a PCI host bridge driver.  It would
> > definitely be nice to move toward a generic, shared driver.  Host
> > bridge drivers are responsible for enumerating the PCI hierarchy below
> > the bridge.  Enumeration is not really PCIe-specific, so I wouldn't
> > put it in drivers/pci/pcie.
> 
> Not a PCI expert, just trying to find out the proper location for this shared driver.
> 
> 
> >> Is using readl/writel IO functions in this driver the best option
> >> which we can have?
> >> Or is there any other recommendation?
> >
> > I'm not really a driver person, but if you're writing a new driver,
> > wouldn't you use the iomap interfaces (ioremap(), ioread32(), etc)
> > rather than readl()?
> 
> That driver exists but it is not in mainline and it is better to directly
> add it to proper location with correct io functions.
> The question is if ioread/iowrite functions are correct one.
> PowerPC io-defs.h suggests that readl/writel should be used for PCI.
> 
> 
> >> Also just want to check if it is correct to use pcie device_type.
> >
> > I don't know what you're asking here.  Can you elaborate or give a
> > specific example?
> 
> If node property device_type pcie should be used.
> device_type = "pcie";
> 
> For pci it is device_type = "pci";
> For network it is device_type = "network"
> the same for serial, watchdogs.
> 
> Do we have any list of device_types which we should use?

Don't use them unless a binding requires it. PCI address lookup code
uses it, so we put it in those nodes. Otherwise device_type is
deprecated and has been for a long time now. You should not be using it
in .dts files.

g.

Re: Sharing PCIE driver between Microblaze and Arm zynq

[Michal Simek]

On 12/07/2012 02:17 PM, Grant Likely wrote:
> On Thu, 6 Dec 2012 15:23:14 +0100, Michal Simek <michal.simek@xilinx.com> wrote:
>> Hi guys,
>>
>> I have a question regarding to sharing generic OF pcie driver between
>> two architectures MB and ARM Zynq.
>> Is drivers/pci/pcie location good for it?
>> Make no sense to have the same driver in two locations.
>> Is using readl/writel IO functions in this driver the best option
>> which we can have?
>> Or is there any other recommendation?

Grant: What do you think about readl/iowrite usage in connection to PCI?

>> Also just want to check if it is correct to use pcie device_type.
>> The rest should be the same with pci description:
>> (http://devicetree.org/MPC5200:PCI#PCI_Address_Translation).
>
> Yes. device_type may be deprecated, but the PCI binding predates that.

If it is deprecated, there could be probably problem with early_init_dt_scan_memory()
which is checking memory device_type (drivers/of/fdt.c).


> PowerPC uses pcie in the device type. arch/powerpc/kernel/of_platform.c

ok. I see.

> Some of that powerpc DT PCI support code really should be made generic.

yep.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: Sharing PCIE driver between Microblaze and Arm zynq

[Arnd Bergmann]

On Friday 07 December 2012, Michal Simek wrote:
> On 12/06/2012 10:27 PM, Bjorn Helgaas wrote:
> > [+cc linux-pci]
> >
> > On Thu, Dec 6, 2012 at 7:23 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> >> Hi guys,
> >>
> >> I have a question regarding to sharing generic OF pcie driver between
> >> two architectures MB and ARM Zynq.
> >> Is drivers/pci/pcie location good for it?
> >> Make no sense to have the same driver in two locations.
> >
> > I think you're talking about a PCI host bridge driver.  It would
> > definitely be nice to move toward a generic, shared driver.  Host
> > bridge drivers are responsible for enumerating the PCI hierarchy below
> > the bridge.  Enumeration is not really PCIe-specific, so I wouldn't
> > put it in drivers/pci/pcie.
> 
> Not a PCI expert, just trying to find out the proper location for this shared driver.

I'd suggest creating a drivers/pci/host directory. We will have more of
these in the future. AFAIK, there is no fully generic (architecture
independent) way to interface a PCI host driver to the PCI subsystem,
but I think we are getting closer to that. You are probably fairly
free to modify the microblaze architecture specific code though if needed.

> >> Is using readl/writel IO functions in this driver the best option
> >> which we can have?
> >> Or is there any other recommendation?
> >
> > I'm not really a driver person, but if you're writing a new driver,
> > wouldn't you use the iomap interfaces (ioremap(), ioread32(), etc)
> > rather than readl()?
> 
> That driver exists but it is not in mainline and it is better to directly
> add it to proper location with correct io functions.
> The question is if ioread/iowrite functions are correct one.
> PowerPC io-defs.h suggests that readl/writel should be used for PCI.

ioread32/iowrite32 are defined to behave the same way readl/writel regarding
addressing modes and barriers, but also allow operating on __iomem pointers
that were returned from ioport_map(). You probably don't need that.

Some architectures like powerpc have their own accessors for on-chip MMIO
areas, but ARM does not. The PowerPC rule is that you must use either
readl/writel or ioread32/iowrite32 to access devices connected to a PCI
bus, but you would not necessarily do that for the PCI host controller
itself on PowerPC.

	Arnd

	Arnd

Re: Sharing PCIE driver between Microblaze and Arm zynq

[Michal Simek]

2012/12/7 Grant Likely <grant.likely@secretlab.ca>:
> On Fri, 07 Dec 2012 10:45:01 +0100, Michal Simek <monstr@monstr.eu> wrote:
>> On 12/06/2012 10:27 PM, Bjorn Helgaas wrote:
>> > [+cc linux-pci]
>> >
>> > On Thu, Dec 6, 2012 at 7:23 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> >> Hi guys,
>> >>
>> >> I have a question regarding to sharing generic OF pcie driver between
>> >> two architectures MB and ARM Zynq.
>> >> Is drivers/pci/pcie location good for it?
>> >> Make no sense to have the same driver in two locations.
>> >
>> > I think you're talking about a PCI host bridge driver.  It would
>> > definitely be nice to move toward a generic, shared driver.  Host
>> > bridge drivers are responsible for enumerating the PCI hierarchy below
>> > the bridge.  Enumeration is not really PCIe-specific, so I wouldn't
>> > put it in drivers/pci/pcie.
>>
>> Not a PCI expert, just trying to find out the proper location for this shared driver.
>>
>>
>> >> Is using readl/writel IO functions in this driver the best option
>> >> which we can have?
>> >> Or is there any other recommendation?
>> >
>> > I'm not really a driver person, but if you're writing a new driver,
>> > wouldn't you use the iomap interfaces (ioremap(), ioread32(), etc)
>> > rather than readl()?
>>
>> That driver exists but it is not in mainline and it is better to directly
>> add it to proper location with correct io functions.
>> The question is if ioread/iowrite functions are correct one.
>> PowerPC io-defs.h suggests that readl/writel should be used for PCI.
>>
>>
>> >> Also just want to check if it is correct to use pcie device_type.
>> >
>> > I don't know what you're asking here.  Can you elaborate or give a
>> > specific example?
>>
>> If node property device_type pcie should be used.
>> device_type = "pcie";
>>
>> For pci it is device_type = "pci";
>> For network it is device_type = "network"
>> the same for serial, watchdogs.
>>
>> Do we have any list of device_types which we should use?
>
> Don't use them unless a binding requires it. PCI address lookup code
> uses it, so we put it in those nodes. Otherwise device_type is
> deprecated and has been for a long time now. You should not be using it
> in .dts files.


Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: Sharing PCIE driver between Microblaze and Arm zynq

[Michal Simek]

2012/12/7 Arnd Bergmann <arnd@arndb.de>:
> On Friday 07 December 2012, Michal Simek wrote:
>> On 12/06/2012 10:27 PM, Bjorn Helgaas wrote:
>> > [+cc linux-pci]
>> >
>> > On Thu, Dec 6, 2012 at 7:23 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> >> Hi guys,
>> >>
>> >> I have a question regarding to sharing generic OF pcie driver between
>> >> two architectures MB and ARM Zynq.
>> >> Is drivers/pci/pcie location good for it?
>> >> Make no sense to have the same driver in two locations.
>> >
>> > I think you're talking about a PCI host bridge driver.  It would
>> > definitely be nice to move toward a generic, shared driver.  Host
>> > bridge drivers are responsible for enumerating the PCI hierarchy below
>> > the bridge.  Enumeration is not really PCIe-specific, so I wouldn't
>> > put it in drivers/pci/pcie.
>>
>> Not a PCI expert, just trying to find out the proper location for this shared driver.
>
> I'd suggest creating a drivers/pci/host directory. We will have more of
> these in the future. AFAIK, there is no fully generic (architecture
> independent) way to interface a PCI host driver to the PCI subsystem,
> but I think we are getting closer to that. You are probably fairly
> free to modify the microblaze architecture specific code though if needed.

yep.


>> >> Is using readl/writel IO functions in this driver the best option
>> >> which we can have?
>> >> Or is there any other recommendation?
>> >
>> > I'm not really a driver person, but if you're writing a new driver,
>> > wouldn't you use the iomap interfaces (ioremap(), ioread32(), etc)
>> > rather than readl()?
>>
>> That driver exists but it is not in mainline and it is better to directly
>> add it to proper location with correct io functions.
>> The question is if ioread/iowrite functions are correct one.
>> PowerPC io-defs.h suggests that readl/writel should be used for PCI.
>
> ioread32/iowrite32 are defined to behave the same way readl/writel regarding
> addressing modes and barriers, but also allow operating on __iomem pointers
> that were returned from ioport_map(). You probably don't need that.
>
> Some architectures like powerpc have their own accessors for on-chip MMIO
> areas, but ARM does not. The PowerPC rule is that you must use either
> readl/writel or ioread32/iowrite32 to access devices connected to a PCI
> bus, but you would not necessarily do that for the PCI host controller
> itself on PowerPC.

ok.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian