Re: Composite PCI devices in FreeBSD (mfd in Linux)

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.

-- Ian



> > 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)
> 
> The only child (ig4(4)) of a given parent lpss device would at most
> need 
> to share access to the middle region, if at all.
> 
> > 
> > Hopefully that gives you a starting point?
> Oh definitely, thanks!  If successful, and the effort to support I2C
> HID 
> devices also comes in, it should enable a bunch of laptops to use
> more 
> stuff like touchscreens and touchpads that are currently broken in 
> FreeBSD (I'm pretty sure one of my laptop's 2 lpss devices is a 
> touchscreen I2C device).
> 
> Anthony
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd
> .org"
>