From owner-freebsd-current@freebsd.org Wed Dec 19 15:41:43 2018 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CD518133971B for ; Wed, 19 Dec 2018 15:41:42 +0000 (UTC) (envelope-from Scoobi_doo@yahoo.com) Received: from sonic307-3.consmr.mail.bf2.yahoo.com (sonic307-3.consmr.mail.bf2.yahoo.com [74.6.134.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id B164386203 for ; Wed, 19 Dec 2018 15:41:41 +0000 (UTC) (envelope-from Scoobi_doo@yahoo.com) X-YMail-OSG: CbmDYQMVM1ky1qXD4smDVmMGulLJODj61q2EFruZ24b3LBe0PqVqDOU7QYGkPt. gJQY8TKIhXs7dRs4eLx1PpkO2bjMFoqVaxqL1M.opYpSzcLNg_9FR_2KQ6QTBqd3VV.lfvyPnN2j W9bmCZ3CrLg4bxej..Vk4fX5ZIzTSoDF3ZpzC0bdcTaq9vxWbfS4C6LjlfiylM4Ek0g5mz4nvUG5 vgEJ3RrgW.Hji1ZDzHJ3YNYWt5c1VghoJV5sHRd4K_LEONXxO8e3SUQIC_MxodNKhW37ci2GHWoM qJfpOjGwCjTjumayGcONw060DxLsKHg3ti9nmyP4kvELN.OO01lRmLRKns0g1mO1PBAdCZZRdpIo ixMbTdmlEUMn3PPK_IWs8BI8PceT5AavolnXi4bbIXckmA00WrD.CxEavpw9409JA.TCFqQaiVxc CVY5L.kDXLcc_WzRiXULg27IHqZYnsyT65fDUZCscRQM4F9X1sNg6gy3G7wInpZtFitjriJ6FZFf r4WFqzfwKHFrId0OnrO70731Um.Fm3L29C8WiIZa5KlUWalUW2t7LcRrYR7R7dndh2lgOYUKToGh olfd3y2JdwEumKm82R5PTnHH9OSweNFjqFSPyYLY1v.DGGvZe_1xo2Mah15Sgw9uNYj700fPrgAn .mByajXZ588nYeDudIG8BoCgg0L3bB_yJJW7k07mopDki9l57QtypWDtfk_dkPDszOlJaTHZatO1 vpwkZrKif7g6vRQOylQlpR8s_AlPeSJZ4WtbiAZVBoZC26jwR7PWGc8e8NGGe1Tv2FEoUfcb0W8_ znBbLxvIMDf6PE7lTLXDPxPU9CPAPBQWLMamY257UCCcEl0oWytYvEEY7CZdxD59TpsRcj7BIryr Jpb91mU7AA4XXmJU3WZz1oSyBWfgJAbtXeDTkdNsKZLrvxz8glylKQx1GIYDt.6WkEX1aso0hzl7 PxLZQiV8bdazc9e0YDgCDZcHBZiB7JnIELge7bTPjUO6QTb.lMyd6CLnCCVvNdeLuTPtqZtHPCAF AxDJUXCGNeyH2hgiuIYtbFHE_9CFZJmNiDFkZiu2AgtACeXIjX48ukKrSBDykWchv9uek6UJ5xGA N_ZQ_qm6VTakb3ssHFupHBKe.WUp9 Received: from sonic.gate.mail.ne1.yahoo.com by sonic307.consmr.mail.bf2.yahoo.com with HTTP; Wed, 19 Dec 2018 15:41:40 +0000 Received: from 192.34.49.8 (EHLO [10.228.144.28]) ([192.34.49.8]) by smtp416.mail.bf1.yahoo.com (Oath Hermes SMTP Server) with ESMTPA ID 7d0bfc118b6fd34b6d2b3451d96b4160; Wed, 19 Dec 2018 15:41:35 +0000 (UTC) Subject: Re: Composite PCI devices in FreeBSD (mfd in Linux) To: John Baldwin , Ian Lepore Cc: FreeBSD CURRENT , Gleb Popov <6yearold@gmail.com> References: <1544473194.1860.340.camel@freebsd.org> <05b1183c-6117-267b-42f9-19e750adfa07@FreeBSD.org> From: Anthony Jenkins Message-ID: <02e30a11-4adf-3efb-d2ce-89c598bda9cf@yahoo.com> Date: Wed, 19 Dec 2018 10:41:34 -0500 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 MIME-Version: 1.0 In-Reply-To: <05b1183c-6117-267b-42f9-19e750adfa07@FreeBSD.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Rspamd-Queue-Id: B164386203 X-Spamd-Bar: + X-Spamd-Result: default: False [1.10 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_SPF_ALLOW(-0.20)[+ptr:yahoo.com]; FREEMAIL_FROM(0.00)[yahoo.com]; RCVD_COUNT_THREE(0.00)[3]; TO_DN_ALL(0.00)[]; MX_GOOD(-0.01)[cached: mta6.am0.yahoodns.net]; DKIM_TRACE(0.00)[yahoo.com:+]; DMARC_POLICY_ALLOW(-0.50)[yahoo.com,reject]; FROM_EQ_ENVFROM(0.00)[]; RCVD_TLS_LAST(0.00)[]; MIME_TRACE(0.00)[0:+]; FREEMAIL_ENVFROM(0.00)[yahoo.com]; ASN(0.00)[asn:26101, ipnet:74.6.128.0/21, country:US]; MID_RHS_MATCH_FROM(0.00)[]; DWL_DNSWL_NONE(0.00)[yahoo.com.dwl.dnswl.org : 127.0.5.0]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.15)[-0.152,0]; R_DKIM_ALLOW(-0.20)[yahoo.com:s=s2048]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; NEURAL_SPAM_SHORT(0.60)[0.598,0]; MIME_GOOD(-0.10)[text/plain]; IP_SCORE(1.35)[ip: (3.82), ipnet: 74.6.128.0/21(1.67), asn: 26101(1.34), country: US(-0.08)]; TO_MATCH_ENVRCPT_SOME(0.00)[]; NEURAL_SPAM_LONG(0.31)[0.313,0]; RCVD_IN_DNSWL_NONE(0.00)[42.134.6.74.list.dnswl.org : 127.0.5.0] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Dec 2018 15:41:43 -0000 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: 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: 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