Date: Wed, 19 Dec 2018 10:41:34 -0500 From: Anthony Jenkins <Scoobi_doo@yahoo.com> To: John Baldwin <jhb@FreeBSD.org>, Ian Lepore <ian@freebsd.org> Cc: FreeBSD CURRENT <freebsd-current@freebsd.org>, Gleb Popov <6yearold@gmail.com> Subject: Re: Composite PCI devices in FreeBSD (mfd in Linux) Message-ID: <02e30a11-4adf-3efb-d2ce-89c598bda9cf@yahoo.com> In-Reply-To: <05b1183c-6117-267b-42f9-19e750adfa07@FreeBSD.org> References: <cf2c24e0-f7d4-9496-7efa-6c5963d77362@yahoo.com> <ff39b848-0444-2018-e206-1cf7486ab19e@FreeBSD.org> <b77e0aab-0b7f-96db-1488-32c92870642c@yahoo.com> <1544473194.1860.340.camel@freebsd.org> <05b1183c-6117-267b-42f9-19e750adfa07@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 12/10/18 3:57 PM, John Baldwin wrote:
> On 12/10/18 12:19 PM, Ian Lepore wrote:
>> On Mon, 2018-12-10 at 14:42 -0500, Anthony Jenkins wrote:
>>> On 12/10/18 1:26 PM, John Baldwin wrote:
>>>> On 12/10/18 9:00 AM, Anthony Jenkins wrote:
>>>>> Hi all,
>>>>>
>>>>> I'm trying to port an Intel PCI I2C controller from Linux to
>>>>> FreeBSD.
>>>>> Linux represents this device as an MFD (multi-function device),
>>>>> meaning
>>>>> it has these "sub-devices" that can be handed off to other
>>>>> drivers to
>>>>> actually attach devices to the system. The Linux "super" PCI
>>>>> device is
>>>>> the intel-lpss-pci.c, and the "sub" device is i2c-designware-
>>>>> platdrv.c,
>>>>> which represents the DesignWare driver's "platform" attachment to
>>>>> the
>>>>> Linux system. FreeBSD also has a DesignWare I2C controller
>>>>> driver,
>>>>> ig4(4), but it only has PCI and ACPI bus attachment
>>>>> implementations.
>>>>>
>>>>> I have a port of the Linux intel-lpss driver to FreeBSD, but now
>>>>> I'm
>>>>> trying to figure out the best way to give FreeBSD's ig4(4) driver
>>>>> access
>>>>> to my lpss(4) device. I'm thinking I could add an ig4_lpss.c
>>>>> describing
>>>>> the "attachment" of an ig4(4) to an lpss(4). Its probe() method
>>>>> would
>>>>> scan the "lpss" devclass for devices, and its attach() method
>>>>> would
>>>>> attach itself as a child to the lpss device and "grab" the
>>>>> portion of
>>>>> PCI memory and the IRQ that the lpss PCI device got.
>>>>>
>>>>> Is this the "FreeBSD Way (TM)" of handling this type of device?
>>>>> If not,
>>>>> can you recommend an existing FreeBSD driver I can model my code
>>>>> after?
>>>>> If my approach is acceptable, how do I fully describe the ig4(4)
>>>>> device's attachment to the system? Is simply making it a child
>>>>> of
>>>>> lpss(4) sufficient? It's "kind of" a PCI device (it is
>>>>> controlled via
>>>>> access to a PCI memory region and an IRQ), but it's a sub-device
>>>>> of an
>>>>> actual PCI device (lpss(4)) attached to PCI.
>>>>> How would my ig4_lpss attachment get information from the lpss(4)
>>>>> driver
>>>>> about what it probed?
>>>> There are some existing PCI drivers that act as "virtual" busses
>>>> that attach
>>>> child devices. For example, vga_pci.c can have drm, agp, and
>>>> acpi_video
>>>> child devices. There are also some SMBus drivers that are also
>>>> PCI-ISA
>>>> bridges and thus create separate child devices.
>>> Yeah I was hoping to avoid using video PCI devices as a model, as
>>> complex as they've gotten recently. I'll check out its bus glue
>>> logic.
>>>
>>>> For a virtual bus like this, you need to figure out how your child
>>>> devices
>>>> will be enumerated. A simple way is to let child devices use an
>>>> identify
>>>> routine that looks at each parent device and decides if a child
>>>> device
>>>> for that driver makes sense. It can then add a child device in the
>>>> identify routine.
>>> Really an lpss parent PCI parent device can only have the following:
>>>
>>> * one of {I2C, UART, SPI} controller
>>> * optionally an IDMA64 controller
>>>
>>> so I was thinking a child ig4(4) device would attach to lpss iff
>>>
>>> * the lpss device detected an I2C controller
>>> * no other ig4 device is already attached
>>>
>>> I haven't fiddled with identify() yet, will look at that tonight.
>>>
>> If this is just another "bus" an ig4 instance can attach to, I'd think
>> the recipe would be to add another DRIVER_MODULE() to ig4_iic.c naming
>> ig4_lpss as the parent. Then add a new ig4_lpss.c modeled after the
>> existing pci and acpi attachment code, its DRIVER_MODULE() would name
>> lpss as parent, and its probe routine would return BUS_PROBE_NOWILDCARD
>> (attach only if specifically added by the parent).
>>
>> Then there would be a new lpss driver that does the resource managment
>> stuff mentioned above, and if it detects configuration for I2C it would
>> do a device_add_child(lpssdev, "ig4_lpss", -1) followed by
>> bus_generic_attach(). There'd be no need for identify() in the child in
>> that case, I think.
>>
>> But take jhb's word over mine on any of this stuff, he's been around
>> since the days when these mechanisms were all invented, whereas I tend
>> to cut and paste that bus and driver attachment stuff in semi-ignorance
>> when I'm working on drivers.
> Doing the device_add_child in the parent driver's attach routine is also
> fine instead of using an identify routine. It's mostly a matter of which
> driver should be in charge of adding the child device (e.g. would you want
> lpss self-contained or should the parent driver know about it explicitly).
>
> If you have an existing ig4 driver you are going to reuse, then you will
> need to ensure you fake up the resource stuff so that the existing code
> works perhaps, though that depends on where the bus_alloc_resource calls
> occur. If they are in shared code you have to fake more. If they are in
> the bus-specific attach routines, then you can just put lpss specific
> logic in the lpss-specific attach routine.
>
>>>> To handle things like resources, you want to have
>>>> bus_*_resource methods that let your child device use the normal
>>>> bus_*
>>>> functions to allocate resources. At the simplest end you don't
>>>> need to
>>>> permit any sharing of BARs among multiple children so you can just
>>>> proxy
>>>> the requests in the "real" PCI driver. (vga_pci.c does this) If
>>>> you need
>>>> the BARs to be shared you have a couple of options such as just
>>>> using a
>>>> refcount on the BAR resource but letting multiple devices allocate
>>>> the same
>>>> BAR. If you want to enforce exclusivity (once a device allocates
>>>> part of
>>>> a BAR then other children shouldn't be permitted to do so), then
>>>> you will
>>>> need a more complicated solution.
>>> Another homework assignment for me - bus_*_resource methods.
>>>
>>> There are 2 or 3 mutually-exclusive sub-regions in the single memory
>>> BAR:
>>>
>>> * 0x000 - 0x200 : I2C sub-device registers
>>> * 0x200 - 0x300 : lpss and I2C sub-device registers
>>> * 0x800 - 0x1000 : IDMA sub-device registers (optional)
> Hmm, so with this arrangement, you could either "cheat" and let all the
> children just use the same BAR, or you could get fancy and create a
> separate rman and sub-allocate resources from that for the different
> ranges that you assign to each child.
So I've cobbled together an ig4_lpss that looks like it attaches to my
fake lpss "bus" and /seems/ to even call the ig4_iic code to attach to
an iicbus, but I don't get a /dev/iic0 device node...
ajenkins@ajenkins-delllaptop ~/Projects/freebsd-intel-lpss (master)
$ dmesg | grep '\(lpss\|ig4\|iic\|pcib0\|ioapic\)'
...
ioapic0: routing intpin 16 (PCI IRQ 16) to lapic 10 vector 52
lpss0: <Intel LPSS PCI Driver> at device 21.0 on pci0
pcib0: allocated type 3 (0xb2000000-0xb2000fff) for rid 10 of lpss0
lpss0: Lazy allocation of 0x1000 bytes rid 0x10 type 3 at 0xb2000000
pcib0: matched entry for 0.21.INTA
pcib0: slot 21 INTA hardwired to IRQ 16
lpss0: IRQ: 0
lpss0: Capabilities: 0x00000200
lpss0: MFP device type: I2C
lpss1: <Intel LPSS PCI Driver> at device 21.1 on pci0
pcib0: allocated type 3 (0xb2001000-0xb2001fff) for rid 10 of lpss1
lpss1: Lazy allocation of 0x1000 bytes rid 0x10 type 3 at 0xb2001000
pcib0: matched entry for 0.21.INTB
pcib0: slot 21 INTB hardwired to IRQ 17
lpss1: IRQ: 0
lpss1: Capabilities: 0x00000201
lpss1: MFP device type: I2C
lpss0: ig4iic_lpss_identify: Entered.
lpss0: lpss_add_child: Entered order=0 name="ig4iic_lpss" unit=-1.
ig4iic_lpss0: ig4iic_lpss_probe: Returning BUS_PROBE_NOWILDCARD.
ig4iic_lpss0: ig4iic_lpss_probe: Returning BUS_PROBE_NOWILDCARD.
ig4iic_lpss0 on lpss0
ig4iic_lpss0: ig4iic_lpss_attach: Entered.
lpss0: lpss_alloc_resource: Entered.
lpss0: lpss_alloc_resource: Returning memory resource 0x0xfffff80005308100.
ig4iic_lpss0: ig4iic_lpss_attach: Got memory resource.
lpss0: lpss_alloc_resource: Entered.
lpss0: lpss_alloc_resource: Returning IRQ resource 0x0xfffff80005308100.
ig4iic_lpss0: ig4iic_lpss_attach: Got interrupt resource.
ig4iic_lpss0: ig4iic_attach: Entered.
ig4iic_lpss0: ig4iic_start: Entered.
ig4iic_lpss0: ig4iic_attach: Returning 0.
ig4iic_lpss0: ig4iic_lpss_attach: Returning 0.
lpss1: ig4iic_lpss_identify: Entered.
lpss1: lpss_add_child: Entered order=0 name="ig4iic_lpss" unit=-1.
ig4iic_lpss1: ig4iic_lpss_probe: Returning BUS_PROBE_NOWILDCARD.
ig4iic_lpss1: ig4iic_lpss_probe: Returning BUS_PROBE_NOWILDCARD.
ig4iic_lpss1 on lpss1
ig4iic_lpss1: ig4iic_lpss_attach: Entered.
lpss1: lpss_alloc_resource: Entered.
lpss1: lpss_alloc_resource: Returning memory resource 0x0xfffff80005308000.
ig4iic_lpss1: ig4iic_lpss_attach: Got memory resource.
lpss1: lpss_alloc_resource: Entered.
lpss1: lpss_alloc_resource: Returning IRQ resource 0x0xfffff80005308000.
ig4iic_lpss1: ig4iic_lpss_attach: Got interrupt resource.
ig4iic_lpss1: ig4iic_attach: Entered.
ioapic0: routing intpin 17 (PCI IRQ 17) to lapic 0 vector 59
ig4iic_lpss1: ig4iic_start: Entered.
ig4iic_lpss1: ig4iic_attach: Returning 0.
ig4iic_lpss1: ig4iic_lpss_attach: Returning 0.
In lpss, my bus_alloc_resource() just forwards the resource handles for
the BAR and IRQ it acquired. In earlier attempts I was hanging the box
hard when I tried to call device_add_child() from lpss' attach method; I
suspect I was getting into an infinite loop adding the same child.
I'm not feeling too confident about the condition of the FreeBSD ig4
driver; the PCI attach code was calling pci_alloc_msi() wrong, passing a
pointer to the rid (0) instead of a pointer to a count variable, and not
passing bus_alloc_resource_any() an IRQ rid > 0 if it has an MSI. I'd
be happy(er) if ig4 created a /dev/iic0 node - I figured iicbus(4) took
care of all that...
https://github.com/ScoobiFreeBSD/freebsd-intel-lpss
Anthony
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?02e30a11-4adf-3efb-d2ce-89c598bda9cf>