All of lore.kernel.org
 help / color / mirror / Atom feed
* Pointers for writing a good PCIe driver
@ 2017-02-06 15:54 Mason
  2017-02-07 15:55 ` Mason
  0 siblings, 1 reply; 14+ messages in thread
From: Mason @ 2017-02-06 15:54 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

Hello,

My platform ( arch/arm/mach-tango ) provides a PCIe controller from PLDA.

I do have access to a driver that works for a few PCIe boards, but it's
an ancient out-of-tree driver (targeting v3.4). Also, I'm not sure it
follows the best-practice guidelines (e.g. it hard-codes a few
work-arounds directly in drivers/pci/probe.c)

If I understand correctly, while PCIe is a standard, it is not codified
as much as USB (XHCI) or SATA (AHCI), which means the register layout
(and possibly other things) are left up to the integrator? Which means
there cannot be some kind of "generic" driver that works for a random
PCIe controller?

I guess my main question is: do you have pointers on getting started
writing a PCIe driver good enough for upstream?

Is Documentation/PCI/PCIEBUS-HOWTO.txt still a relevant resource?

Are there good resources to get up to speed on the PCIe terminology?

Regards.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-06 15:54 Pointers for writing a good PCIe driver Mason
@ 2017-02-07 15:55 ` Mason
  2017-02-07 20:06   ` Bjorn Helgaas
  0 siblings, 1 reply; 14+ messages in thread
From: Mason @ 2017-02-07 15:55 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

On 06/02/2017 16:54, Mason wrote:

> My platform ( arch/arm/mach-tango ) provides a PCIe controller from PLDA.
> 
> I do have access to a driver that works for a few PCIe boards, but it's
> an ancient out-of-tree driver (targeting v3.4). Also, I'm not sure it
> follows the best-practice guidelines (e.g. it hard-codes a few
> work-arounds directly in drivers/pci/probe.c)
> 
> If I understand correctly, while PCIe is a standard, it is not codified
> as much as USB (XHCI) or SATA (AHCI), which means the register layout
> (and possibly other things) are left up to the integrator? Which means
> there cannot be some kind of "generic" driver that works for a random
> PCIe controller?
> 
> I guess my main question is: do you have pointers on getting started
> writing a PCIe driver good enough for upstream?
> 
> Is Documentation/PCI/PCIEBUS-HOWTO.txt still a relevant resource?
> 
> Are there good resources to get up to speed on the PCIe terminology?

I'm also wondering: in many Linux subsystems, there are often
several drivers of different quality; some are obsolescent and
have not been updated in a while, while some are recent and
follow all the latest best-practice guidelines (and the rest
is somewhere in the middle).

Are there 1 or 2 very good PCIe drivers to take as examples
as good starting points? (Perhaps a simple driver, and a
more complex driver.)

Does anyone know if any SoC with a driver upstream is using
a PLDA controller?

Regards.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-07 15:55 ` Mason
@ 2017-02-07 20:06   ` Bjorn Helgaas
  2017-02-07 21:47     ` Bjorn Helgaas
  0 siblings, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2017-02-07 20:06 UTC (permalink / raw)
  To: Mason; +Cc: Bjorn Helgaas, linux-pci

On Tue, Feb 07, 2017 at 04:55:28PM +0100, Mason wrote:
> On 06/02/2017 16:54, Mason wrote:
> 
> > My platform ( arch/arm/mach-tango ) provides a PCIe controller from PLDA.
> > 
> > I do have access to a driver that works for a few PCIe boards, but it's
> > an ancient out-of-tree driver (targeting v3.4). Also, I'm not sure it
> > follows the best-practice guidelines (e.g. it hard-codes a few
> > work-arounds directly in drivers/pci/probe.c)

Indeed, having to change drivers/pci/probe.c doesn't sound like a best
practice :)  If you can share details of what changes you need, people
could probably suggest other ways to do it within the generic PCI
framework.

> > If I understand correctly, while PCIe is a standard, it is not codified
> > as much as USB (XHCI) or SATA (AHCI), which means the register layout
> > (and possibly other things) are left up to the integrator? Which means
> > there cannot be some kind of "generic" driver that works for a random
> > PCIe controller?

There is no standardization of registers in MMIO or I/O port space.
The size, location, and type of those registers is configurable in a
generic way via the BARs, of course.  But that tells you nothing about
the *functionality* of those MMIO or I/O port registers.

For config space, the first 64 bytes (the PCIe r3.0 spec calls it the
"PCI 3.0 Compatible Configuration Space Header") are completely
standardized.  This is where device IDs, BARs, and other generic
registers are.

PCIe devices can implement up to 4096 bytes of config space.  The
space after the 64-byte header can contain a list of standardized
PCI/PCIe Capabilities, non-standardized device-specific registers, or
both.  There is a way to include device-specific registers in a
"Vendor Defined" Capability -- that way drivers can use generic
mechanisms to find registers so they don't have to hard-code register
layout assumptions.

> > I guess my main question is: do you have pointers on getting started
> > writing a PCIe driver good enough for upstream?
> > 
> > Is Documentation/PCI/PCIEBUS-HOWTO.txt still a relevant resource?

PCIEBUS-HOWTO.txt is still relevant, but only to drivers for services
of PCIe ports, e.g., hotplug, error reporting, etc.  It's not relevant
to PCIe endpoint drivers.

> > Are there good resources to get up to speed on the PCIe terminology?
> 
> I'm also wondering: in many Linux subsystems, there are often
> several drivers of different quality; some are obsolescent and
> have not been updated in a while, while some are recent and
> follow all the latest best-practice guidelines (and the rest
> is somewhere in the middle).
> 
> Are there 1 or 2 very good PCIe drivers to take as examples
> as good starting points? (Perhaps a simple driver, and a
> more complex driver.)

I'm not really the best person to ask about this because I deal more
with the PCI core than with the drivers that use the core.  If I were
looking, I might start with drivers/nvme/host/pci.c or
drivers/usb/host/xhci-pci.c.

Bjorn

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-07 20:06   ` Bjorn Helgaas
@ 2017-02-07 21:47     ` Bjorn Helgaas
  2017-02-07 22:56       ` Mason
  0 siblings, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2017-02-07 21:47 UTC (permalink / raw)
  To: Mason; +Cc: Bjorn Helgaas, linux-pci

On Tue, Feb 07, 2017 at 02:06:56PM -0600, Bjorn Helgaas wrote:
> On Tue, Feb 07, 2017 at 04:55:28PM +0100, Mason wrote:
> > On 06/02/2017 16:54, Mason wrote:
> > 
> > > My platform ( arch/arm/mach-tango ) provides a PCIe controller from PLDA.
> > > 
> > > I do have access to a driver that works for a few PCIe boards, but it's
> > > an ancient out-of-tree driver (targeting v3.4). Also, I'm not sure it
> > > follows the best-practice guidelines (e.g. it hard-codes a few
> > > work-arounds directly in drivers/pci/probe.c)
> 
> Indeed, having to change drivers/pci/probe.c doesn't sound like a best
> practice :)  If you can share details of what changes you need, people
> could probably suggest other ways to do it within the generic PCI
> framework.
> 
> > > If I understand correctly, while PCIe is a standard, it is not codified
> > > as much as USB (XHCI) or SATA (AHCI), which means the register layout
> > > (and possibly other things) are left up to the integrator? Which means
> > > there cannot be some kind of "generic" driver that works for a random
> > > PCIe controller?
> 
> There is no standardization of registers in MMIO or I/O port space.
> The size, location, and type of those registers is configurable in a
> generic way via the BARs, of course.  But that tells you nothing about
> the *functionality* of those MMIO or I/O port registers.
> 
> For config space, the first 64 bytes (the PCIe r3.0 spec calls it the
> "PCI 3.0 Compatible Configuration Space Header") are completely
> standardized.  This is where device IDs, BARs, and other generic
> registers are.
> 
> PCIe devices can implement up to 4096 bytes of config space.  The
> space after the 64-byte header can contain a list of standardized
> PCI/PCIe Capabilities, non-standardized device-specific registers, or
> both.  There is a way to include device-specific registers in a
> "Vendor Defined" Capability -- that way drivers can use generic
> mechanisms to find registers so they don't have to hard-code register
> layout assumptions.
> 
> > > I guess my main question is: do you have pointers on getting started
> > > writing a PCIe driver good enough for upstream?
> > > 
> > > Is Documentation/PCI/PCIEBUS-HOWTO.txt still a relevant resource?
> 
> PCIEBUS-HOWTO.txt is still relevant, but only to drivers for services
> of PCIe ports, e.g., hotplug, error reporting, etc.  It's not relevant
> to PCIe endpoint drivers.
> 
> > > Are there good resources to get up to speed on the PCIe terminology?
> > 
> > I'm also wondering: in many Linux subsystems, there are often
> > several drivers of different quality; some are obsolescent and
> > have not been updated in a while, while some are recent and
> > follow all the latest best-practice guidelines (and the rest
> > is somewhere in the middle).
> > 
> > Are there 1 or 2 very good PCIe drivers to take as examples
> > as good starting points? (Perhaps a simple driver, and a
> > more complex driver.)
> 
> I'm not really the best person to ask about this because I deal more
> with the PCI core than with the drivers that use the core.  If I were
> looking, I might start with drivers/nvme/host/pci.c or
> drivers/usb/host/xhci-pci.c.

I should have also mentioned Documentation/PCI/pci.txt.  That's a
pretty good place to start.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-07 21:47     ` Bjorn Helgaas
@ 2017-02-07 22:56       ` Mason
  2017-02-12 16:50         ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Mason @ 2017-02-07 22:56 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Bjorn Helgaas, linux-pci

On 07/02/2017 22:47, Bjorn Helgaas wrote:
> On Tue, Feb 07, 2017 at 02:06:56PM -0600, Bjorn Helgaas wrote:
>> On Tue, Feb 07, 2017 at 04:55:28PM +0100, Mason wrote:
>>> On 06/02/2017 16:54, Mason wrote:
>>>
>>>> My platform ( arch/arm/mach-tango ) provides a PCIe controller from PLDA.
>>>>
>>>> I do have access to a driver that works for a few PCIe boards, but it's
>>>> an ancient out-of-tree driver (targeting v3.4). Also, I'm not sure it
>>>> follows the best-practice guidelines (e.g. it hard-codes a few
>>>> work-arounds directly in drivers/pci/probe.c)
>>
>> Indeed, having to change drivers/pci/probe.c doesn't sound like a best
>> practice :)  If you can share details of what changes you need, people
>> could probably suggest other ways to do it within the generic PCI
>> framework.
>>
>>>> If I understand correctly, while PCIe is a standard, it is not codified
>>>> as much as USB (XHCI) or SATA (AHCI), which means the register layout
>>>> (and possibly other things) are left up to the integrator? Which means
>>>> there cannot be some kind of "generic" driver that works for a random
>>>> PCIe controller?
>>
>> There is no standardization of registers in MMIO or I/O port space.
>> The size, location, and type of those registers is configurable in a
>> generic way via the BARs, of course.  But that tells you nothing about
>> the *functionality* of those MMIO or I/O port registers.
>>
>> For config space, the first 64 bytes (the PCIe r3.0 spec calls it the
>> "PCI 3.0 Compatible Configuration Space Header") are completely
>> standardized.  This is where device IDs, BARs, and other generic
>> registers are.
>>
>> PCIe devices can implement up to 4096 bytes of config space.  The
>> space after the 64-byte header can contain a list of standardized
>> PCI/PCIe Capabilities, non-standardized device-specific registers, or
>> both.  There is a way to include device-specific registers in a
>> "Vendor Defined" Capability -- that way drivers can use generic
>> mechanisms to find registers so they don't have to hard-code register
>> layout assumptions.
>>
>>>> I guess my main question is: do you have pointers on getting started
>>>> writing a PCIe driver good enough for upstream?
>>>>
>>>> Is Documentation/PCI/PCIEBUS-HOWTO.txt still a relevant resource?
>>
>> PCIEBUS-HOWTO.txt is still relevant, but only to drivers for services
>> of PCIe ports, e.g., hotplug, error reporting, etc.  It's not relevant
>> to PCIe endpoint drivers.
>>
>>>> Are there good resources to get up to speed on the PCIe terminology?
>>>
>>> I'm also wondering: in many Linux subsystems, there are often
>>> several drivers of different quality; some are obsolescent and
>>> have not been updated in a while, while some are recent and
>>> follow all the latest best-practice guidelines (and the rest
>>> is somewhere in the middle).
>>>
>>> Are there 1 or 2 very good PCIe drivers to take as examples
>>> as good starting points? (Perhaps a simple driver, and a
>>> more complex driver.)
>>
>> I'm not really the best person to ask about this because I deal more
>> with the PCI core than with the drivers that use the core.  If I were
>> looking, I might start with drivers/nvme/host/pci.c or
>> drivers/usb/host/xhci-pci.c.
> 
> I should have also mentioned Documentation/PCI/pci.txt.  That's a
> pretty good place to start.

Hello Bjorn,

Thanks for these pointers, I will study them with care.

Who would review my PCIe driver if I ever submitted one? :-)

Regards.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-07 22:56       ` Mason
@ 2017-02-12 16:50         ` Greg KH
  2017-02-17  9:20           ` Mason
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2017-02-12 16:50 UTC (permalink / raw)
  To: Mason; +Cc: Bjorn Helgaas, Bjorn Helgaas, linux-pci

On Tue, Feb 07, 2017 at 11:56:31PM +0100, Mason wrote:
> On 07/02/2017 22:47, Bjorn Helgaas wrote:
> > On Tue, Feb 07, 2017 at 02:06:56PM -0600, Bjorn Helgaas wrote:
> >> On Tue, Feb 07, 2017 at 04:55:28PM +0100, Mason wrote:
> >>> On 06/02/2017 16:54, Mason wrote:
> >>>
> >>>> My platform ( arch/arm/mach-tango ) provides a PCIe controller from PLDA.
> >>>>
> >>>> I do have access to a driver that works for a few PCIe boards, but it's
> >>>> an ancient out-of-tree driver (targeting v3.4). Also, I'm not sure it
> >>>> follows the best-practice guidelines (e.g. it hard-codes a few
> >>>> work-arounds directly in drivers/pci/probe.c)
> >>
> >> Indeed, having to change drivers/pci/probe.c doesn't sound like a best
> >> practice :)  If you can share details of what changes you need, people
> >> could probably suggest other ways to do it within the generic PCI
> >> framework.
> >>
> >>>> If I understand correctly, while PCIe is a standard, it is not codified
> >>>> as much as USB (XHCI) or SATA (AHCI), which means the register layout
> >>>> (and possibly other things) are left up to the integrator? Which means
> >>>> there cannot be some kind of "generic" driver that works for a random
> >>>> PCIe controller?
> >>
> >> There is no standardization of registers in MMIO or I/O port space.
> >> The size, location, and type of those registers is configurable in a
> >> generic way via the BARs, of course.  But that tells you nothing about
> >> the *functionality* of those MMIO or I/O port registers.
> >>
> >> For config space, the first 64 bytes (the PCIe r3.0 spec calls it the
> >> "PCI 3.0 Compatible Configuration Space Header") are completely
> >> standardized.  This is where device IDs, BARs, and other generic
> >> registers are.
> >>
> >> PCIe devices can implement up to 4096 bytes of config space.  The
> >> space after the 64-byte header can contain a list of standardized
> >> PCI/PCIe Capabilities, non-standardized device-specific registers, or
> >> both.  There is a way to include device-specific registers in a
> >> "Vendor Defined" Capability -- that way drivers can use generic
> >> mechanisms to find registers so they don't have to hard-code register
> >> layout assumptions.
> >>
> >>>> I guess my main question is: do you have pointers on getting started
> >>>> writing a PCIe driver good enough for upstream?
> >>>>
> >>>> Is Documentation/PCI/PCIEBUS-HOWTO.txt still a relevant resource?
> >>
> >> PCIEBUS-HOWTO.txt is still relevant, but only to drivers for services
> >> of PCIe ports, e.g., hotplug, error reporting, etc.  It's not relevant
> >> to PCIe endpoint drivers.
> >>
> >>>> Are there good resources to get up to speed on the PCIe terminology?
> >>>
> >>> I'm also wondering: in many Linux subsystems, there are often
> >>> several drivers of different quality; some are obsolescent and
> >>> have not been updated in a while, while some are recent and
> >>> follow all the latest best-practice guidelines (and the rest
> >>> is somewhere in the middle).
> >>>
> >>> Are there 1 or 2 very good PCIe drivers to take as examples
> >>> as good starting points? (Perhaps a simple driver, and a
> >>> more complex driver.)
> >>
> >> I'm not really the best person to ask about this because I deal more
> >> with the PCI core than with the drivers that use the core.  If I were
> >> looking, I might start with drivers/nvme/host/pci.c or
> >> drivers/usb/host/xhci-pci.c.
> > 
> > I should have also mentioned Documentation/PCI/pci.txt.  That's a
> > pretty good place to start.
> 
> Hello Bjorn,
> 
> Thanks for these pointers, I will study them with care.
> 
> Who would review my PCIe driver if I ever submitted one? :-)

It all depends on what subsystem it would belong to, what type of device
are you writing a driver for?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-12 16:50         ` Greg KH
@ 2017-02-17  9:20           ` Mason
  2017-02-17 14:38             ` Bjorn Helgaas
  0 siblings, 1 reply; 14+ messages in thread
From: Mason @ 2017-02-17  9:20 UTC (permalink / raw)
  To: Greg KH; +Cc: Bjorn Helgaas, Bjorn Helgaas, linux-pci

On 12/02/2017 17:50, Greg KH wrote:
> On Tue, Feb 07, 2017 at 11:56:31PM +0100, Mason wrote:
>> On 07/02/2017 22:47, Bjorn Helgaas wrote:
>>> On Tue, Feb 07, 2017 at 02:06:56PM -0600, Bjorn Helgaas wrote:
>>>> On Tue, Feb 07, 2017 at 04:55:28PM +0100, Mason wrote:
>>>>> On 06/02/2017 16:54, Mason wrote:
>>>>>
>>>>>> My platform ( arch/arm/mach-tango ) provides a PCIe controller from PLDA.
>>>>>>
>>>>>> I do have access to a driver that works for a few PCIe boards, but it's
>>>>>> an ancient out-of-tree driver (targeting v3.4). Also, I'm not sure it
>>>>>> follows the best-practice guidelines (e.g. it hard-codes a few
>>>>>> work-arounds directly in drivers/pci/probe.c)
>>>>
>>>> Indeed, having to change drivers/pci/probe.c doesn't sound like a best
>>>> practice :)  If you can share details of what changes you need, people
>>>> could probably suggest other ways to do it within the generic PCI
>>>> framework.
>>>>
>>>>>> If I understand correctly, while PCIe is a standard, it is not codified
>>>>>> as much as USB (XHCI) or SATA (AHCI), which means the register layout
>>>>>> (and possibly other things) are left up to the integrator? Which means
>>>>>> there cannot be some kind of "generic" driver that works for a random
>>>>>> PCIe controller?
>>>>
>>>> There is no standardization of registers in MMIO or I/O port space.
>>>> The size, location, and type of those registers is configurable in a
>>>> generic way via the BARs, of course.  But that tells you nothing about
>>>> the *functionality* of those MMIO or I/O port registers.
>>>>
>>>> For config space, the first 64 bytes (the PCIe r3.0 spec calls it the
>>>> "PCI 3.0 Compatible Configuration Space Header") are completely
>>>> standardized.  This is where device IDs, BARs, and other generic
>>>> registers are.
>>>>
>>>> PCIe devices can implement up to 4096 bytes of config space.  The
>>>> space after the 64-byte header can contain a list of standardized
>>>> PCI/PCIe Capabilities, non-standardized device-specific registers, or
>>>> both.  There is a way to include device-specific registers in a
>>>> "Vendor Defined" Capability -- that way drivers can use generic
>>>> mechanisms to find registers so they don't have to hard-code register
>>>> layout assumptions.
>>>>
>>>>>> I guess my main question is: do you have pointers on getting started
>>>>>> writing a PCIe driver good enough for upstream?
>>>>>>
>>>>>> Is Documentation/PCI/PCIEBUS-HOWTO.txt still a relevant resource?
>>>>
>>>> PCIEBUS-HOWTO.txt is still relevant, but only to drivers for services
>>>> of PCIe ports, e.g., hotplug, error reporting, etc.  It's not relevant
>>>> to PCIe endpoint drivers.
>>>>
>>>>>> Are there good resources to get up to speed on the PCIe terminology?
>>>>>
>>>>> I'm also wondering: in many Linux subsystems, there are often
>>>>> several drivers of different quality; some are obsolescent and
>>>>> have not been updated in a while, while some are recent and
>>>>> follow all the latest best-practice guidelines (and the rest
>>>>> is somewhere in the middle).
>>>>>
>>>>> Are there 1 or 2 very good PCIe drivers to take as examples
>>>>> as good starting points? (Perhaps a simple driver, and a
>>>>> more complex driver.)
>>>>
>>>> I'm not really the best person to ask about this because I deal more
>>>> with the PCI core than with the drivers that use the core.  If I were
>>>> looking, I might start with drivers/nvme/host/pci.c or
>>>> drivers/usb/host/xhci-pci.c.
>>>
>>> I should have also mentioned Documentation/PCI/pci.txt.  That's a
>>> pretty good place to start.
>>
>> Hello Bjorn,
>>
>> Thanks for these pointers, I will study them with care.
>>
>> Who would review my PCIe driver if I ever submitted one? :-)
> 
> It all depends on what subsystem it would belong to, what type of device
> are you writing a driver for?

Hello Greg,

There might be some kind of misunderstanding.

I don't plan to write a driver for a device that plugs into a PCIe slot.

I intend to write a driver for the PCIe controller itself, which is
embedded in the SoC. (In order to support different types of PCIe
devices, with their respective drivers already upstream.)

I think the controller driver belongs in drivers/pci/host ?

Regards.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-17  9:20           ` Mason
@ 2017-02-17 14:38             ` Bjorn Helgaas
  2017-02-17 17:00                 ` Mason
  0 siblings, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2017-02-17 14:38 UTC (permalink / raw)
  To: Mason; +Cc: Greg KH, Bjorn Helgaas, linux-pci

On Fri, Feb 17, 2017 at 10:20:20AM +0100, Mason wrote:
> On 12/02/2017 17:50, Greg KH wrote:
> > On Tue, Feb 07, 2017 at 11:56:31PM +0100, Mason wrote:
> >> On 07/02/2017 22:47, Bjorn Helgaas wrote:
> >>> On Tue, Feb 07, 2017 at 02:06:56PM -0600, Bjorn Helgaas wrote:
> >>>> On Tue, Feb 07, 2017 at 04:55:28PM +0100, Mason wrote:
> >>>>> On 06/02/2017 16:54, Mason wrote:
> >>>>>
> >>>>>> My platform ( arch/arm/mach-tango ) provides a PCIe controller from PLDA.
> >>>>>>
> >>>>>> I do have access to a driver that works for a few PCIe boards, but it's
> >>>>>> an ancient out-of-tree driver (targeting v3.4). Also, I'm not sure it
> >>>>>> follows the best-practice guidelines (e.g. it hard-codes a few
> >>>>>> work-arounds directly in drivers/pci/probe.c)
> >>>>
> >>>> Indeed, having to change drivers/pci/probe.c doesn't sound like a best
> >>>> practice :)  If you can share details of what changes you need, people
> >>>> could probably suggest other ways to do it within the generic PCI
> >>>> framework.
> >>>>
> >>>>>> If I understand correctly, while PCIe is a standard, it is not codified
> >>>>>> as much as USB (XHCI) or SATA (AHCI), which means the register layout
> >>>>>> (and possibly other things) are left up to the integrator? Which means
> >>>>>> there cannot be some kind of "generic" driver that works for a random
> >>>>>> PCIe controller?
> >>>>
> >>>> There is no standardization of registers in MMIO or I/O port space.
> >>>> The size, location, and type of those registers is configurable in a
> >>>> generic way via the BARs, of course.  But that tells you nothing about
> >>>> the *functionality* of those MMIO or I/O port registers.
> >>>>
> >>>> For config space, the first 64 bytes (the PCIe r3.0 spec calls it the
> >>>> "PCI 3.0 Compatible Configuration Space Header") are completely
> >>>> standardized.  This is where device IDs, BARs, and other generic
> >>>> registers are.
> >>>>
> >>>> PCIe devices can implement up to 4096 bytes of config space.  The
> >>>> space after the 64-byte header can contain a list of standardized
> >>>> PCI/PCIe Capabilities, non-standardized device-specific registers, or
> >>>> both.  There is a way to include device-specific registers in a
> >>>> "Vendor Defined" Capability -- that way drivers can use generic
> >>>> mechanisms to find registers so they don't have to hard-code register
> >>>> layout assumptions.
> >>>>
> >>>>>> I guess my main question is: do you have pointers on getting started
> >>>>>> writing a PCIe driver good enough for upstream?
> >>>>>>
> >>>>>> Is Documentation/PCI/PCIEBUS-HOWTO.txt still a relevant resource?
> >>>>
> >>>> PCIEBUS-HOWTO.txt is still relevant, but only to drivers for services
> >>>> of PCIe ports, e.g., hotplug, error reporting, etc.  It's not relevant
> >>>> to PCIe endpoint drivers.
> >>>>
> >>>>>> Are there good resources to get up to speed on the PCIe terminology?
> >>>>>
> >>>>> I'm also wondering: in many Linux subsystems, there are often
> >>>>> several drivers of different quality; some are obsolescent and
> >>>>> have not been updated in a while, while some are recent and
> >>>>> follow all the latest best-practice guidelines (and the rest
> >>>>> is somewhere in the middle).
> >>>>>
> >>>>> Are there 1 or 2 very good PCIe drivers to take as examples
> >>>>> as good starting points? (Perhaps a simple driver, and a
> >>>>> more complex driver.)
> >>>>
> >>>> I'm not really the best person to ask about this because I deal more
> >>>> with the PCI core than with the drivers that use the core.  If I were
> >>>> looking, I might start with drivers/nvme/host/pci.c or
> >>>> drivers/usb/host/xhci-pci.c.
> >>>
> >>> I should have also mentioned Documentation/PCI/pci.txt.  That's a
> >>> pretty good place to start.
> >>
> >> Hello Bjorn,
> >>
> >> Thanks for these pointers, I will study them with care.
> >>
> >> Who would review my PCIe driver if I ever submitted one? :-)
> > 
> > It all depends on what subsystem it would belong to, what type of device
> > are you writing a driver for?
> 
> Hello Greg,
> 
> There might be some kind of misunderstanding.
> 
> I don't plan to write a driver for a device that plugs into a PCIe slot.
> 
> I intend to write a driver for the PCIe controller itself, which is
> embedded in the SoC. (In order to support different types of PCIe
> devices, with their respective drivers already upstream.)
> 
> I think the controller driver belongs in drivers/pci/host ?

Sorry, I indeed misunderstood you.

The native PCI host controller drivers indeed live in
drivers/pci/host/.

I don't know anything about your hardware or environment, but I highly
encourage you to use ACPI (drivers/acpi/pci_root.c, with a little bit
of arch support) or generic DT (drivers/pci/host/pci-host-generic.c)
instead of writing a custom host controller driver.

The native drivers in drivers/pci/host are a huge maintenance hassle
for no real benefit.

Bjorn

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-17 14:38             ` Bjorn Helgaas
@ 2017-02-17 17:00                 ` Mason
  0 siblings, 0 replies; 14+ messages in thread
From: Mason @ 2017-02-17 17:00 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Greg KH, Bjorn Helgaas, linux-pci, Linux ARM

On 17/02/2017 15:38, Bjorn Helgaas wrote:

> On Fri, Feb 17, 2017 at 10:20:20AM +0100, Mason wrote:
>
>> There might be some kind of misunderstanding.
>>
>> I don't plan to write a driver for a device that plugs into a PCIe slot.
>>
>> I intend to write a driver for the PCIe controller itself, which is
>> embedded in the SoC. (In order to support different types of PCIe
>> devices, with their respective drivers already upstream.)
>>
>> I think the controller driver belongs in drivers/pci/host ?
> 
> Sorry, I indeed misunderstood you.
> 
> The native PCI host controller drivers indeed live in
> drivers/pci/host/.
> 
> I don't know anything about your hardware or environment, but I highly
> encourage you to use ACPI (drivers/acpi/pci_root.c, with a little bit
> of arch support) or generic DT (drivers/pci/host/pci-host-generic.c)
> instead of writing a custom host controller driver.
> 
> The native drivers in drivers/pci/host are a huge maintenance hassle
> for no real benefit.

My SoC is built around an ARM Cortex A9.
I don't think (?) this platform has any support for ACPI.

I'm currently using this DT description:
http://lxr.free-electrons.com/source/arch/arm/boot/dts/tango4-common.dtsi

The "legacy" driver sits at ~700 lines. It would be awesome to be able
to discard all of it, and rely on generic upstream code. But I don't
understand how it is possible for a generic driver to support whatever
crazy solution some random vendor has come up with?

Regards.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Pointers for writing a good PCIe driver
@ 2017-02-17 17:00                 ` Mason
  0 siblings, 0 replies; 14+ messages in thread
From: Mason @ 2017-02-17 17:00 UTC (permalink / raw)
  To: linux-arm-kernel

On 17/02/2017 15:38, Bjorn Helgaas wrote:

> On Fri, Feb 17, 2017 at 10:20:20AM +0100, Mason wrote:
>
>> There might be some kind of misunderstanding.
>>
>> I don't plan to write a driver for a device that plugs into a PCIe slot.
>>
>> I intend to write a driver for the PCIe controller itself, which is
>> embedded in the SoC. (In order to support different types of PCIe
>> devices, with their respective drivers already upstream.)
>>
>> I think the controller driver belongs in drivers/pci/host ?
> 
> Sorry, I indeed misunderstood you.
> 
> The native PCI host controller drivers indeed live in
> drivers/pci/host/.
> 
> I don't know anything about your hardware or environment, but I highly
> encourage you to use ACPI (drivers/acpi/pci_root.c, with a little bit
> of arch support) or generic DT (drivers/pci/host/pci-host-generic.c)
> instead of writing a custom host controller driver.
> 
> The native drivers in drivers/pci/host are a huge maintenance hassle
> for no real benefit.

My SoC is built around an ARM Cortex A9.
I don't think (?) this platform has any support for ACPI.

I'm currently using this DT description:
http://lxr.free-electrons.com/source/arch/arm/boot/dts/tango4-common.dtsi

The "legacy" driver sits at ~700 lines. It would be awesome to be able
to discard all of it, and rely on generic upstream code. But I don't
understand how it is possible for a generic driver to support whatever
crazy solution some random vendor has come up with?

Regards.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-17 17:00                 ` Mason
@ 2017-02-17 17:50                   ` Bjorn Helgaas
  -1 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2017-02-17 17:50 UTC (permalink / raw)
  To: Mason; +Cc: Bjorn Helgaas, Greg KH, Linux ARM, linux-pci

On Fri, Feb 17, 2017 at 06:00:53PM +0100, Mason wrote:
> On 17/02/2017 15:38, Bjorn Helgaas wrote:
> 
> > On Fri, Feb 17, 2017 at 10:20:20AM +0100, Mason wrote:
> >
> >> There might be some kind of misunderstanding.
> >>
> >> I don't plan to write a driver for a device that plugs into a PCIe slot.
> >>
> >> I intend to write a driver for the PCIe controller itself, which is
> >> embedded in the SoC. (In order to support different types of PCIe
> >> devices, with their respective drivers already upstream.)
> >>
> >> I think the controller driver belongs in drivers/pci/host ?
> > 
> > Sorry, I indeed misunderstood you.
> > 
> > The native PCI host controller drivers indeed live in
> > drivers/pci/host/.
> > 
> > I don't know anything about your hardware or environment, but I highly
> > encourage you to use ACPI (drivers/acpi/pci_root.c, with a little bit
> > of arch support) or generic DT (drivers/pci/host/pci-host-generic.c)
> > instead of writing a custom host controller driver.
> > 
> > The native drivers in drivers/pci/host are a huge maintenance hassle
> > for no real benefit.
> 
> My SoC is built around an ARM Cortex A9.
> I don't think (?) this platform has any support for ACPI.

There's ACPI support on ARM64, but none that I know of for ARM32.

> I'm currently using this DT description:
> http://lxr.free-electrons.com/source/arch/arm/boot/dts/tango4-common.dtsi
> 
> The "legacy" driver sits at ~700 lines. It would be awesome to be able
> to discard all of it, and rely on generic upstream code. But I don't
> understand how it is possible for a generic driver to support whatever
> crazy solution some random vendor has come up with?

You're right that the programming model of the host bridge itself is
not specified by PCI specs, so it's impossible to have a generic
driver that can manage it completely by itself.

If you have firmware that initializes the bridge and tells the OS what
the windows are (bus numbers, memory space, I/O space) and where the
PCIe-specified ECAM space is, a generic driver can take it from there.

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Pointers for writing a good PCIe driver
@ 2017-02-17 17:50                   ` Bjorn Helgaas
  0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2017-02-17 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 17, 2017 at 06:00:53PM +0100, Mason wrote:
> On 17/02/2017 15:38, Bjorn Helgaas wrote:
> 
> > On Fri, Feb 17, 2017 at 10:20:20AM +0100, Mason wrote:
> >
> >> There might be some kind of misunderstanding.
> >>
> >> I don't plan to write a driver for a device that plugs into a PCIe slot.
> >>
> >> I intend to write a driver for the PCIe controller itself, which is
> >> embedded in the SoC. (In order to support different types of PCIe
> >> devices, with their respective drivers already upstream.)
> >>
> >> I think the controller driver belongs in drivers/pci/host ?
> > 
> > Sorry, I indeed misunderstood you.
> > 
> > The native PCI host controller drivers indeed live in
> > drivers/pci/host/.
> > 
> > I don't know anything about your hardware or environment, but I highly
> > encourage you to use ACPI (drivers/acpi/pci_root.c, with a little bit
> > of arch support) or generic DT (drivers/pci/host/pci-host-generic.c)
> > instead of writing a custom host controller driver.
> > 
> > The native drivers in drivers/pci/host are a huge maintenance hassle
> > for no real benefit.
> 
> My SoC is built around an ARM Cortex A9.
> I don't think (?) this platform has any support for ACPI.

There's ACPI support on ARM64, but none that I know of for ARM32.

> I'm currently using this DT description:
> http://lxr.free-electrons.com/source/arch/arm/boot/dts/tango4-common.dtsi
> 
> The "legacy" driver sits at ~700 lines. It would be awesome to be able
> to discard all of it, and rely on generic upstream code. But I don't
> understand how it is possible for a generic driver to support whatever
> crazy solution some random vendor has come up with?

You're right that the programming model of the host bridge itself is
not specified by PCI specs, so it's impossible to have a generic
driver that can manage it completely by itself.

If you have firmware that initializes the bridge and tells the OS what
the windows are (bus numbers, memory space, I/O space) and where the
PCIe-specified ECAM space is, a generic driver can take it from there.

Bjorn

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Pointers for writing a good PCIe driver
  2017-02-17 17:50                   ` Bjorn Helgaas
@ 2017-02-20 16:12                     ` Mason
  -1 siblings, 0 replies; 14+ messages in thread
From: Mason @ 2017-02-20 16:12 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Greg KH, Bjorn Helgaas, linux-pci, Linux ARM, Thibaud Cornic,
	Phuong Nguyen, Will Deacon, David Daney, Rob Herring,
	Thierry Reding

On 17/02/2017 18:50, Bjorn Helgaas wrote:

> On Fri, Feb 17, 2017 at 06:00:53PM +0100, Mason wrote:
>
>> On 17/02/2017 15:38, Bjorn Helgaas wrote:
>>
>>> The native PCI host controller drivers indeed live in drivers/pci/host/
>>>
>>> I don't know anything about your hardware or environment, but I highly
>>> encourage you to use ACPI (drivers/acpi/pci_root.c, with a little bit
>>> of arch support) or generic DT (drivers/pci/host/pci-host-generic.c)
>>> instead of writing a custom host controller driver.
>>>
>>> The native drivers in drivers/pci/host are a huge maintenance hassle
>>> for no real benefit.
>>
>> I'm currently using this DT description:
>> http://lxr.free-electrons.com/source/arch/arm/boot/dts/tango4-common.dtsi
>>
>> The "legacy" driver sits at ~700 lines. It would be awesome to be able
>> to discard all of it, and rely on generic upstream code. But I don't
>> understand how it is possible for a generic driver to support whatever
>> crazy solution some random vendor has come up with?
> 
> You're right that the programming model of the host bridge itself is
> not specified by PCI specs, so it's impossible to have a generic
> driver that can manage it completely by itself.
> 
> If you have firmware that initializes the bridge and tells the OS what
> the windows are (bus numbers, memory space, I/O space) and where the
> PCIe-specified ECAM space is, a generic driver can take it from there.

(Mostly walking myself through the code and documentation,
noting references along the way.)

I took a look at drivers/pci/host/pci-host-generic.c
which is a tiny file, so the magic must be happening elsewhere.
=> drivers/pci/host/pci-host-common.c (for starters)

gen_pci_cfg_cam_bus_ops and pci_generic_ecam_ops have the same
.pci_ops, but different .bus_shift

$ git grep pci-host-cam-generic
Documentation/devicetree/bindings/pci/host-generic-pci.txt:- compatible     : Must be "pci-host-cam-generic" or "pci-host-ecam-generic"
Documentation/devicetree/bindings/pci/host-generic-pci.txt:    compatible = "pci-host-cam-generic"
drivers/pci/host/pci-host-generic.c:    { .compatible = "pci-host-cam-generic",

$ git grep pci-host-ecam-generic
Documentation/devicetree/bindings/pci/host-generic-pci.txt:- compatible     : Must be "pci-host-cam-generic" or "pci-host-ecam-generic"
arch/arm/boot/dts/alpine.dtsi:                  compatible = "pci-host-ecam-generic";
arch/arm64/boot/dts/al/alpine-v2.dtsi:                  compatible = "pci-host-ecam-generic";
arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi:                   compatible = "pci-host-ecam-generic";
arch/arm64/boot/dts/arm/juno-base.dtsi:         compatible = "arm,juno-r1-pcie", "plda,xpressrich3-axi", "pci-host-ecam-generic";
arch/arm64/boot/dts/broadcom/vulcan.dtsi:               compatible = "pci-host-ecam-generic";
drivers/pci/host/pci-host-generic.c:    { .compatible = "pci-host-ecam-generic",

https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt

> Firmware-initialised PCI host controllers and PCI emulations, such as the
> virtio-pci implementations found in kvmtool and other para-virtualised
> systems, do not require driver support for complexities such as regulator
> and clock management. In fact, the controller may not even require the
> configuration of a control interface by the operating system, instead
> presenting a set of fixed windows describing a subset of IO, Memory and
> Configuration Spaces.

For the only arm32 user, arch/arm/boot/dts/alpine.dtsi:

		/* Internal PCIe Controller */
		pcie-internal@0xfbc00000 {
			compatible = "pci-host-ecam-generic";
			device_type = "pci";
			#size-cells = <2>;
			#address-cells = <3>;
			#interrupt-cells = <1>;
			reg = <0x0 0xfbc00000 0x0 0x100000>;
			interrupt-map-mask = <0xf800 0 0 7>;
			/* Add legacy interrupts for SATA devices only */
			interrupt-map =	<0x4000 0 0 1 &gic 0 43 4>,
					<0x4800 0 0 1 &gic 0 44 4>;

			/* 32 bit non prefetchable memory space */
			ranges = <0x02000000 0x0 0xfe000000 0x0 0xfe000000 0x0 0x1000000>;

			bus-range = <0x00 0x00>;
			msi-parent = <&msix>;
		};


Compatible string depends on "the layout of configuration space
(CAM vs ECAM respectively)"

https://en.wikipedia.org/wiki/PCI_configuration_space


The register interface to my PCIe controller is:

0x2e000	REGION_0_BASE_REG
0x2e004	REGION_1_BASE_REG
0x2e008	REGION_2_BASE_REG
0x2e00c	REGION_3_BASE_REG
0x2e010	REGION_4_BASE_REG
0x2e014	REGION_5_BASE_REG
0x2e018	REGION_6_BASE_REG
0x2e01c	REGION_7_BASE_REG
0x2e020	DMA_RD_MBUS_ADDR_REG
0x2e024	DMA_RD_ADDR_LO_REG
0x2e028	DMA_RD_ADDR_HI_REG
0x2e02c	DMA_RD_CNT_REG
0x2e030	DMA_WR_MBUS_ADDR_REG
0x2e034	DMA_WR_ADDR_LO_REG
0x2e038	DMA_WR_ADDR_HI_REG
0x2e03c	DMA_WR_CNT_REG
0x2e040	DMA_RD_EN_REG
0x2e044	DMA_WR_EN_REG
0x2e048	PCI_ADDR_LO_BASE_REG
0x2e04c	PCI_ADDR_HI_BASE_REG
0x2e050	PM_REG
0x2e054	MAX_CPL_TIMEOUT_REG
0x2e058	CORE_CONF_0_REG
0x2e05c	CORE_CONF_1_REG
0x2e060	CBA_ADDR_REG
0x2e064	CBA_DATA_REG
0x2e068	DEBUG_0_REG
0x2e06c	DEBUG_1_REG
0x2e070	DEBUG_2_REG
0x2e074	CORE_TEST_OUT_REG
0x2e078	INT_NOT_MSI_REG
0x2e07c	MSI_REG
0x2e080	MSI_0_REG
0x2e084	MSI_1_REG
0x2e088	MSI_2_REG
0x2e08c	MSI_3_REG
0x2e090	MSI_4_REG
0x2e094	MSI_5_REG
0x2e098	MSI_6_REG
0x2e09c	MSI_7_REG
0x2e0a0	MSI_0_MASK_REG
0x2e0a4	MSI_1_MASK_REG
0x2e0a8	MSI_2_MASK_REG
0x2e0ac	MSI_3_MASK_REG
0x2e0b0	MSI_4_MASK_REG
0x2e0b4	MSI_5_MASK_REG
0x2e0b8	MSI_6_MASK_REG
0x2e0bc	MSI_7_MASK_REG
0x2e0c0	PHY_HW_TRAPS_REG
0x2e0c4	COMMON_BIST_0_REG
0x2e0c8	COMMON_BIST_1_REG
0x2e0cc	LANE_0_BIST_REG
0x2e0d0	CUSTOM_CONF_LINK_REG
0x2e0d4	CUSTOM_CONF_LANE_0_REG
0x2e0e0	PLL_SETTING_REG
0x2e0e4	DEBUG_3_REG
0x2e0e8	DEBUG_4_REG
0x2e0ec	DEBUG_5_REG


In the "legacy" driver (for kernel v3.4), I see

static struct pci_ops tangox_pcie_ops = {
	.read	= tangox_pcie_read_conf,
	.write	= tangox_pcie_write_conf,
};

#define TANGOX_PCIE_ENABLE_CONFIG_IO()      WR_PCIE_REG32(TANGOX_PCIE_PCI_ADDR_LO_BASE_REG, ((TANGOX_PCIE_ADDR_BASE & 0xff000000) | 0x1))
#define TANGOX_PCIE_DISABLE_CONFIG_IO()     WR_PCIE_REG32(TANGOX_PCIE_PCI_ADDR_LO_BASE_REG, ((TANGOX_PCIE_ADDR_BASE & 0xff000000) & ~0x1))

static int tangox_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
                                 int where, int size, u32 *val)
{
    void __iomem *addr;

    TANGOX_PCIE_ENABLE_CONFIG_IO();

    addr =  (void __iomem *)(
            tangox_pcie.regs +
            PCIE_CONF_BUS(bus->number) +
            PCIE_CONF_DEV(PCI_SLOT(devfn)) +
            PCIE_CONF_FUNC(PCI_FUNC(devfn)) +
            PCIE_CONF_REG(where));

    *val = readl( addr );

	if (size == 1)
		*val = (*val >> (8 * (where & 3))) & 0xff;
	else if (size == 2)
		*val = (*val >> (8 * (where & 3))) & 0xffff;

    TANGOX_PCIE_DISABLE_CONFIG_IO();

    return PCIBIOS_SUCCESSFUL;
}


I see that pci-host-common.c calls:
pci_scan_root_bus(dev, cfg->busr.start, &ops->pci_ops, cfg, &resources);

whereas the legacy driver calls:
pci_scan_root_bus(NULL, sys->busnr, &tangox_pcie_ops, sys, &sys->resources);

So the generic equivalent of
tangox_pcie_read_conf() and tangox_pcie_write_conf()
should be
pci_generic_config_read() and pci_generic_config_write()


int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
			    int where, int size, u32 *val)
{
	void __iomem *addr;

	addr = bus->ops->map_bus(bus, devfn, where);
	if (!addr) {
		*val = ~0;
		return PCIBIOS_DEVICE_NOT_FOUND;
	}

	if (size == 1)
		*val = readb(addr);
	else if (size == 2)
		*val = readw(addr);
	else
		*val = readl(addr);

	return PCIBIOS_SUCCESSFUL;
}


Problem might come from the
TANGOX_PCIE_ENABLE_CONFIG_IO() / TANGOX_PCIE_DISABLE_CONFIG_IO()
shenanigans.

Also, I see that the legacy implementation reads 32 bits,
then performs the shift & mask in software. I'm hoping that
the ARM core does the right thing for readb/readw.


Taking a closer look at TANGOX_PCIE_ENABLE_CONFIG_IO()
WR_PCIE_REG32(TANGOX_PCIE_PCI_ADDR_LO_BASE_REG, ((TANGOX_PCIE_ADDR_BASE & 0xff000000) | 0x1))

Register "PCI_ADDR_LO_BASE_REG" is composed of
pci_addr_base_lo[31:28] and pci_conf[0]

  pci_conf : if 1, the access by LBUS is a configuration request. If 0, the access by LBUS is a memory request.
  pci_addr_base_lo : lower part of the PCI address for access by LBUS.

So it looks like I need to fudge something, perhaps override map_bus method?

What is a "memory request", in PCIe lingo?
In other words, when do I need to clear the "pci_conf" bit?
(Maybe firmware can just set it to 1, and be done with it?)

Regards.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Pointers for writing a good PCIe driver
@ 2017-02-20 16:12                     ` Mason
  0 siblings, 0 replies; 14+ messages in thread
From: Mason @ 2017-02-20 16:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 17/02/2017 18:50, Bjorn Helgaas wrote:

> On Fri, Feb 17, 2017 at 06:00:53PM +0100, Mason wrote:
>
>> On 17/02/2017 15:38, Bjorn Helgaas wrote:
>>
>>> The native PCI host controller drivers indeed live in drivers/pci/host/
>>>
>>> I don't know anything about your hardware or environment, but I highly
>>> encourage you to use ACPI (drivers/acpi/pci_root.c, with a little bit
>>> of arch support) or generic DT (drivers/pci/host/pci-host-generic.c)
>>> instead of writing a custom host controller driver.
>>>
>>> The native drivers in drivers/pci/host are a huge maintenance hassle
>>> for no real benefit.
>>
>> I'm currently using this DT description:
>> http://lxr.free-electrons.com/source/arch/arm/boot/dts/tango4-common.dtsi
>>
>> The "legacy" driver sits at ~700 lines. It would be awesome to be able
>> to discard all of it, and rely on generic upstream code. But I don't
>> understand how it is possible for a generic driver to support whatever
>> crazy solution some random vendor has come up with?
> 
> You're right that the programming model of the host bridge itself is
> not specified by PCI specs, so it's impossible to have a generic
> driver that can manage it completely by itself.
> 
> If you have firmware that initializes the bridge and tells the OS what
> the windows are (bus numbers, memory space, I/O space) and where the
> PCIe-specified ECAM space is, a generic driver can take it from there.

(Mostly walking myself through the code and documentation,
noting references along the way.)

I took a look at drivers/pci/host/pci-host-generic.c
which is a tiny file, so the magic must be happening elsewhere.
=> drivers/pci/host/pci-host-common.c (for starters)

gen_pci_cfg_cam_bus_ops and pci_generic_ecam_ops have the same
.pci_ops, but different .bus_shift

$ git grep pci-host-cam-generic
Documentation/devicetree/bindings/pci/host-generic-pci.txt:- compatible     : Must be "pci-host-cam-generic" or "pci-host-ecam-generic"
Documentation/devicetree/bindings/pci/host-generic-pci.txt:    compatible = "pci-host-cam-generic"
drivers/pci/host/pci-host-generic.c:    { .compatible = "pci-host-cam-generic",

$ git grep pci-host-ecam-generic
Documentation/devicetree/bindings/pci/host-generic-pci.txt:- compatible     : Must be "pci-host-cam-generic" or "pci-host-ecam-generic"
arch/arm/boot/dts/alpine.dtsi:                  compatible = "pci-host-ecam-generic";
arch/arm64/boot/dts/al/alpine-v2.dtsi:                  compatible = "pci-host-ecam-generic";
arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi:                   compatible = "pci-host-ecam-generic";
arch/arm64/boot/dts/arm/juno-base.dtsi:         compatible = "arm,juno-r1-pcie", "plda,xpressrich3-axi", "pci-host-ecam-generic";
arch/arm64/boot/dts/broadcom/vulcan.dtsi:               compatible = "pci-host-ecam-generic";
drivers/pci/host/pci-host-generic.c:    { .compatible = "pci-host-ecam-generic",

https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt

> Firmware-initialised PCI host controllers and PCI emulations, such as the
> virtio-pci implementations found in kvmtool and other para-virtualised
> systems, do not require driver support for complexities such as regulator
> and clock management. In fact, the controller may not even require the
> configuration of a control interface by the operating system, instead
> presenting a set of fixed windows describing a subset of IO, Memory and
> Configuration Spaces.

For the only arm32 user, arch/arm/boot/dts/alpine.dtsi:

		/* Internal PCIe Controller */
		pcie-internal at 0xfbc00000 {
			compatible = "pci-host-ecam-generic";
			device_type = "pci";
			#size-cells = <2>;
			#address-cells = <3>;
			#interrupt-cells = <1>;
			reg = <0x0 0xfbc00000 0x0 0x100000>;
			interrupt-map-mask = <0xf800 0 0 7>;
			/* Add legacy interrupts for SATA devices only */
			interrupt-map =	<0x4000 0 0 1 &gic 0 43 4>,
					<0x4800 0 0 1 &gic 0 44 4>;

			/* 32 bit non prefetchable memory space */
			ranges = <0x02000000 0x0 0xfe000000 0x0 0xfe000000 0x0 0x1000000>;

			bus-range = <0x00 0x00>;
			msi-parent = <&msix>;
		};


Compatible string depends on "the layout of configuration space
(CAM vs ECAM respectively)"

https://en.wikipedia.org/wiki/PCI_configuration_space


The register interface to my PCIe controller is:

0x2e000	REGION_0_BASE_REG
0x2e004	REGION_1_BASE_REG
0x2e008	REGION_2_BASE_REG
0x2e00c	REGION_3_BASE_REG
0x2e010	REGION_4_BASE_REG
0x2e014	REGION_5_BASE_REG
0x2e018	REGION_6_BASE_REG
0x2e01c	REGION_7_BASE_REG
0x2e020	DMA_RD_MBUS_ADDR_REG
0x2e024	DMA_RD_ADDR_LO_REG
0x2e028	DMA_RD_ADDR_HI_REG
0x2e02c	DMA_RD_CNT_REG
0x2e030	DMA_WR_MBUS_ADDR_REG
0x2e034	DMA_WR_ADDR_LO_REG
0x2e038	DMA_WR_ADDR_HI_REG
0x2e03c	DMA_WR_CNT_REG
0x2e040	DMA_RD_EN_REG
0x2e044	DMA_WR_EN_REG
0x2e048	PCI_ADDR_LO_BASE_REG
0x2e04c	PCI_ADDR_HI_BASE_REG
0x2e050	PM_REG
0x2e054	MAX_CPL_TIMEOUT_REG
0x2e058	CORE_CONF_0_REG
0x2e05c	CORE_CONF_1_REG
0x2e060	CBA_ADDR_REG
0x2e064	CBA_DATA_REG
0x2e068	DEBUG_0_REG
0x2e06c	DEBUG_1_REG
0x2e070	DEBUG_2_REG
0x2e074	CORE_TEST_OUT_REG
0x2e078	INT_NOT_MSI_REG
0x2e07c	MSI_REG
0x2e080	MSI_0_REG
0x2e084	MSI_1_REG
0x2e088	MSI_2_REG
0x2e08c	MSI_3_REG
0x2e090	MSI_4_REG
0x2e094	MSI_5_REG
0x2e098	MSI_6_REG
0x2e09c	MSI_7_REG
0x2e0a0	MSI_0_MASK_REG
0x2e0a4	MSI_1_MASK_REG
0x2e0a8	MSI_2_MASK_REG
0x2e0ac	MSI_3_MASK_REG
0x2e0b0	MSI_4_MASK_REG
0x2e0b4	MSI_5_MASK_REG
0x2e0b8	MSI_6_MASK_REG
0x2e0bc	MSI_7_MASK_REG
0x2e0c0	PHY_HW_TRAPS_REG
0x2e0c4	COMMON_BIST_0_REG
0x2e0c8	COMMON_BIST_1_REG
0x2e0cc	LANE_0_BIST_REG
0x2e0d0	CUSTOM_CONF_LINK_REG
0x2e0d4	CUSTOM_CONF_LANE_0_REG
0x2e0e0	PLL_SETTING_REG
0x2e0e4	DEBUG_3_REG
0x2e0e8	DEBUG_4_REG
0x2e0ec	DEBUG_5_REG


In the "legacy" driver (for kernel v3.4), I see

static struct pci_ops tangox_pcie_ops = {
	.read	= tangox_pcie_read_conf,
	.write	= tangox_pcie_write_conf,
};

#define TANGOX_PCIE_ENABLE_CONFIG_IO()      WR_PCIE_REG32(TANGOX_PCIE_PCI_ADDR_LO_BASE_REG, ((TANGOX_PCIE_ADDR_BASE & 0xff000000) | 0x1))
#define TANGOX_PCIE_DISABLE_CONFIG_IO()     WR_PCIE_REG32(TANGOX_PCIE_PCI_ADDR_LO_BASE_REG, ((TANGOX_PCIE_ADDR_BASE & 0xff000000) & ~0x1))

static int tangox_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
                                 int where, int size, u32 *val)
{
    void __iomem *addr;

    TANGOX_PCIE_ENABLE_CONFIG_IO();

    addr =  (void __iomem *)(
            tangox_pcie.regs +
            PCIE_CONF_BUS(bus->number) +
            PCIE_CONF_DEV(PCI_SLOT(devfn)) +
            PCIE_CONF_FUNC(PCI_FUNC(devfn)) +
            PCIE_CONF_REG(where));

    *val = readl( addr );

	if (size == 1)
		*val = (*val >> (8 * (where & 3))) & 0xff;
	else if (size == 2)
		*val = (*val >> (8 * (where & 3))) & 0xffff;

    TANGOX_PCIE_DISABLE_CONFIG_IO();

    return PCIBIOS_SUCCESSFUL;
}


I see that pci-host-common.c calls:
pci_scan_root_bus(dev, cfg->busr.start, &ops->pci_ops, cfg, &resources);

whereas the legacy driver calls:
pci_scan_root_bus(NULL, sys->busnr, &tangox_pcie_ops, sys, &sys->resources);

So the generic equivalent of
tangox_pcie_read_conf() and tangox_pcie_write_conf()
should be
pci_generic_config_read() and pci_generic_config_write()


int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
			    int where, int size, u32 *val)
{
	void __iomem *addr;

	addr = bus->ops->map_bus(bus, devfn, where);
	if (!addr) {
		*val = ~0;
		return PCIBIOS_DEVICE_NOT_FOUND;
	}

	if (size == 1)
		*val = readb(addr);
	else if (size == 2)
		*val = readw(addr);
	else
		*val = readl(addr);

	return PCIBIOS_SUCCESSFUL;
}


Problem might come from the
TANGOX_PCIE_ENABLE_CONFIG_IO() / TANGOX_PCIE_DISABLE_CONFIG_IO()
shenanigans.

Also, I see that the legacy implementation reads 32 bits,
then performs the shift & mask in software. I'm hoping that
the ARM core does the right thing for readb/readw.


Taking a closer look at TANGOX_PCIE_ENABLE_CONFIG_IO()
WR_PCIE_REG32(TANGOX_PCIE_PCI_ADDR_LO_BASE_REG, ((TANGOX_PCIE_ADDR_BASE & 0xff000000) | 0x1))

Register "PCI_ADDR_LO_BASE_REG" is composed of
pci_addr_base_lo[31:28] and pci_conf[0]

  pci_conf : if 1, the access by LBUS is a configuration request. If 0, the access by LBUS is a memory request.
  pci_addr_base_lo : lower part of the PCI address for access by LBUS.

So it looks like I need to fudge something, perhaps override map_bus method?

What is a "memory request", in PCIe lingo?
In other words, when do I need to clear the "pci_conf" bit?
(Maybe firmware can just set it to 1, and be done with it?)

Regards.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-02-20 16:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-06 15:54 Pointers for writing a good PCIe driver Mason
2017-02-07 15:55 ` Mason
2017-02-07 20:06   ` Bjorn Helgaas
2017-02-07 21:47     ` Bjorn Helgaas
2017-02-07 22:56       ` Mason
2017-02-12 16:50         ` Greg KH
2017-02-17  9:20           ` Mason
2017-02-17 14:38             ` Bjorn Helgaas
2017-02-17 17:00               ` Mason
2017-02-17 17:00                 ` Mason
2017-02-17 17:50                 ` Bjorn Helgaas
2017-02-17 17:50                   ` Bjorn Helgaas
2017-02-20 16:12                   ` Mason
2017-02-20 16:12                     ` Mason

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.