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
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to