| Message ID | 20220221162652.103834-11-clement.leger@bootlin.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | add support for fwnode in i2c mux system and sfp | expand |
On Mon, Feb 21, 2022 at 05:26:52PM +0100, Clément Léger wrote: > Add support to retrieve a i2c bus in sfp with a fwnode. This support > is using the fwnode API which also works with device-tree and ACPI. > For this purpose, the device-tree and ACPI code handling the i2c > adapter retrieval was factorized with the new code. This also allows > i2c devices using a software_node description to be used by sfp code. > > Signed-off-by: Clément Léger <clement.leger@bootlin.com> I think this looks fine. Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Thanks.
On Mon, Feb 21, 2022 at 05:26:52PM +0100, Clément Léger wrote: > Add support to retrieve a i2c bus in sfp with a fwnode. This support > is using the fwnode API which also works with device-tree and ACPI. > For this purpose, the device-tree and ACPI code handling the i2c > adapter retrieval was factorized with the new code. This also allows > i2c devices using a software_node description to be used by sfp code. If I'm not mistaken this patch can even go separately right now, since all used APIs are already available.
Le Mon, 21 Feb 2022 19:57:39 +0200, Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : > On Mon, Feb 21, 2022 at 05:26:52PM +0100, Clément Léger wrote: > > Add support to retrieve a i2c bus in sfp with a fwnode. This support > > is using the fwnode API which also works with device-tree and ACPI. > > For this purpose, the device-tree and ACPI code handling the i2c > > adapter retrieval was factorized with the new code. This also allows > > i2c devices using a software_node description to be used by sfp code. > > If I'm not mistaken this patch can even go separately right now, since all used > APIs are already available. > This patches uses fwnode_find_i2c_adapter_by_node() which is introduced by "i2c: fwnode: add fwnode_find_i2c_adapter_by_node()" but they can probably be contributed both in a separate series.
On Tue, Feb 22, 2022 at 02:25:13PM +0100, Clément Léger wrote: > Le Mon, 21 Feb 2022 19:57:39 +0200, > Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : > > > On Mon, Feb 21, 2022 at 05:26:52PM +0100, Clément Léger wrote: > > > Add support to retrieve a i2c bus in sfp with a fwnode. This support > > > is using the fwnode API which also works with device-tree and ACPI. > > > For this purpose, the device-tree and ACPI code handling the i2c > > > adapter retrieval was factorized with the new code. This also allows > > > i2c devices using a software_node description to be used by sfp code. > > > > If I'm not mistaken this patch can even go separately right now, since all used > > APIs are already available. > > This patches uses fwnode_find_i2c_adapter_by_node() which is introduced > by "i2c: fwnode: add fwnode_find_i2c_adapter_by_node()" but they can > probably be contributed both in a separate series. I summon Hans into the discussion since I remember he recently refactored a bit I2C (ACPI/fwnode) APIs. Also he might have an idea about entire big picture approach with this series based on his ACPI experience.
Hi, On 2/23/22 12:22, Andy Shevchenko wrote: > On Tue, Feb 22, 2022 at 02:25:13PM +0100, Clément Léger wrote: >> Le Mon, 21 Feb 2022 19:57:39 +0200, >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : >> >>> On Mon, Feb 21, 2022 at 05:26:52PM +0100, Clément Léger wrote: >>>> Add support to retrieve a i2c bus in sfp with a fwnode. This support >>>> is using the fwnode API which also works with device-tree and ACPI. >>>> For this purpose, the device-tree and ACPI code handling the i2c >>>> adapter retrieval was factorized with the new code. This also allows >>>> i2c devices using a software_node description to be used by sfp code. >>> >>> If I'm not mistaken this patch can even go separately right now, since all used >>> APIs are already available. >> >> This patches uses fwnode_find_i2c_adapter_by_node() which is introduced >> by "i2c: fwnode: add fwnode_find_i2c_adapter_by_node()" but they can >> probably be contributed both in a separate series. > > I summon Hans into the discussion since I remember he recently refactored > a bit I2C (ACPI/fwnode) APIs. Also he might have an idea about entire big > picture approach with this series based on his ACPI experience. If I understand this series correctly then this is about a PCI-E card which has an I2C controller on the card and behind that I2C-controller there are a couple if I2C muxes + I2C clients. And the goal of the series is to describe those I2C muxes + I2C clients with software nodes so that the existing I2C enumeration code can be used (after porting the existing I2C enumeration code from OF functions to generic fwnode functions). Did I understand this bit correctly? I believe that a lot of the discussion here is caused by the initial problem / hw-setup this series tries to address / support is not described very well ? Assuming I did understand the above correctly. One alternative would be to simply manually instantiate the I2C muxes + clients using i2c_new_client_device(). But I'm not sure if i2c_new_client_device() will work for the muxes without adding some software_nodes which brings us back to something like this patch-set. In general I believe that porting things away from OF specific parsing to the generic fwnode APIs is a good thing. Making device_get_match_data() for devices with only a software fwnode use of_device_id matching feels a bit weird. But it also makes sense since that requires just adding a compatible string to the software fwnode properties which is easy and it allows re-uses existing matching code in the drivers. I understand various people falling over this weirdness but AFAICT the alternative would be adding some special swnode_id type + matching code for devices where the primary fwnode is a software fwnode, which would just be a whole bunch of extra code ending up with something similar. So re-using of_device_id-s for matching of devices where the primary fwnode is a software fwnode seems like a good idea. *But* this all needs to be explained in the commit message a lot better. It really needs to be spelled out that this is: a) Only for matching devices where the primary fwnode is a software fwnode b) Really has nothing to do with of/dt but of_device_id matching is used here to avoid having to introduce a new matching mechanism just for devices where the primary fwnode is a software fwnode c) That introducing a new software fwnode matching mechanism would be a bad idea since this will require adding new swnode_match tables to many drivers, where as re-using of_device_id will make drivers which already have an of_match_table just work. And this should be spelled out in both the commit message as well as in some documentation / kdoc comments. Because although a useful trick, reusing the of_match_id-s is also confusing which I believe is what has led to a lot of the discussion on this patch-set so far. Note the above all relies on my interpretation of this patch set, which may be wrong, since as said the patch-set does seem to be lacking when it comes to documentation / motivation of the patches. Regards, Hans
> If I understand this series correctly then this is about a PCI-E card > which has an I2C controller on the card and behind that I2C-controller > there are a couple if I2C muxes + I2C clients. They are not i2c clients in the normal sense. The i2c bus connects to an SFP cage. You can hot plug different sort of network modules into the cage. So for example fibre optic modules or copper modules. The modules have an 'at24' like block of memory, which is a mix of EEPROM and status values. For copper modules, there is an MDIO over I2C protocol which allows access to the Copper Ethernet PHY inside the module. The current device tree binding is that you have a node for the SFP cage, which includes a phandle to the i2c bus, plus a list of GPIOs connected to pins on the SFP cage. The SFP driver will then directly access the memory, maybe instantiate an mdio-over-i2c device if needed, and control the GPIOs. The Ethernet driver then has an OF node with a phandle pointing to the SFP device. The whole design of this is based around the hardware being a collection of standard parts, i2c bus, i2c mux, gpio controller, ethernet controller, each with their own driver as part of a linux subsystem, and then having some glue to put all the parts together. And at the moment, that glue is DT. > Note the above all relies on my interpretation of this patch set, > which may be wrong, since as said the patch-set does seem to be > lacking when it comes to documentation / motivation of the patches. I think some examples from DT will help with this. Andrew
On Wed, Feb 23, 2022 at 01:02:23PM +0100, Hans de Goede wrote: > Hi, > > On 2/23/22 12:22, Andy Shevchenko wrote: > > On Tue, Feb 22, 2022 at 02:25:13PM +0100, Clément Léger wrote: > >> Le Mon, 21 Feb 2022 19:57:39 +0200, > >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : > >> > >>> On Mon, Feb 21, 2022 at 05:26:52PM +0100, Clément Léger wrote: > >>>> Add support to retrieve a i2c bus in sfp with a fwnode. This support > >>>> is using the fwnode API which also works with device-tree and ACPI. > >>>> For this purpose, the device-tree and ACPI code handling the i2c > >>>> adapter retrieval was factorized with the new code. This also allows > >>>> i2c devices using a software_node description to be used by sfp code. > >>> > >>> If I'm not mistaken this patch can even go separately right now, since all used > >>> APIs are already available. > >> > >> This patches uses fwnode_find_i2c_adapter_by_node() which is introduced > >> by "i2c: fwnode: add fwnode_find_i2c_adapter_by_node()" but they can > >> probably be contributed both in a separate series. > > > > I summon Hans into the discussion since I remember he recently refactored > > a bit I2C (ACPI/fwnode) APIs. Also he might have an idea about entire big > > picture approach with this series based on his ACPI experience. > > If I understand this series correctly then this is about a PCI-E card > which has an I2C controller on the card and behind that I2C-controller > there are a couple if I2C muxes + I2C clients. That is what I gathered as well. > Assuming I did understand the above correctly. One alternative would be > to simply manually instantiate the I2C muxes + clients using > i2c_new_client_device(). But I'm not sure if i2c_new_client_device() > will work for the muxes without adding some software_nodes which > brings us back to something like this patch-set. That assumes that an I2C device is always present, which is not always the case - there are hot-pluggable devices on I2C buses. Specifically, this series includes pluggable SFP modules, which fall into this category of "hot-pluggable I2C devices" - spanning several bus addresses (0x50, 0x51, 0x56). 0x50 is EEPROM like, but not quite as the top 128 bytes is paged and sometimes buggy in terms of access behaviour. 0x51 contains a bunch of monitoring and other controls for the module which again can be paged. At 0x56, there may possibly be some kind of device that translates I2C accesses to MDIO accesses to access a PHY onboard. Consequently, the SFP driver and MDIO translation layer wants access to the I2C bus, rather than a device. Now, before ARM was converted to DT, we had ways to cope with non-firmware described setups like this by using platform devices and platform data. Much of that ended up deprecated, because - hey - DT is great and more modern and the old way is disgusting and we want to get rid of it. However, that approach locks us into describing stuff in firmware, which is unsuitable when something like this comes along. I think what we need is both approaches. We need a way for the SFP driver (which is a platform_driver) to be used _without_ needing descriptions in firmware. I think we have that for GPIOs, but for an I2C bus, We have i2c_get_adapter() for I2C buses, but that needs the bus number - we could either pass the i2c_adapter or the adapter number through platform data to the SFP driver. Or is there another solution to being able to reuse multi-driver based infrastructure that we have developed based on DT descriptions in situations such as an add-in PCI card?
Hi, On 2/23/22 13:41, Russell King (Oracle) wrote: > On Wed, Feb 23, 2022 at 01:02:23PM +0100, Hans de Goede wrote: >> Hi, >> >> On 2/23/22 12:22, Andy Shevchenko wrote: >>> On Tue, Feb 22, 2022 at 02:25:13PM +0100, Clément Léger wrote: >>>> Le Mon, 21 Feb 2022 19:57:39 +0200, >>>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : >>>> >>>>> On Mon, Feb 21, 2022 at 05:26:52PM +0100, Clément Léger wrote: >>>>>> Add support to retrieve a i2c bus in sfp with a fwnode. This support >>>>>> is using the fwnode API which also works with device-tree and ACPI. >>>>>> For this purpose, the device-tree and ACPI code handling the i2c >>>>>> adapter retrieval was factorized with the new code. This also allows >>>>>> i2c devices using a software_node description to be used by sfp code. >>>>> >>>>> If I'm not mistaken this patch can even go separately right now, since all used >>>>> APIs are already available. >>>> >>>> This patches uses fwnode_find_i2c_adapter_by_node() which is introduced >>>> by "i2c: fwnode: add fwnode_find_i2c_adapter_by_node()" but they can >>>> probably be contributed both in a separate series. >>> >>> I summon Hans into the discussion since I remember he recently refactored >>> a bit I2C (ACPI/fwnode) APIs. Also he might have an idea about entire big >>> picture approach with this series based on his ACPI experience. >> >> If I understand this series correctly then this is about a PCI-E card >> which has an I2C controller on the card and behind that I2C-controller >> there are a couple if I2C muxes + I2C clients. > > That is what I gathered as well. > >> Assuming I did understand the above correctly. One alternative would be >> to simply manually instantiate the I2C muxes + clients using >> i2c_new_client_device(). But I'm not sure if i2c_new_client_device() >> will work for the muxes without adding some software_nodes which >> brings us back to something like this patch-set. > > That assumes that an I2C device is always present, which is not always > the case - there are hot-pluggable devices on I2C buses. > > Specifically, this series includes pluggable SFP modules, which fall > into this category of "hot-pluggable I2C devices" - spanning several > bus addresses (0x50, 0x51, 0x56). 0x50 is EEPROM like, but not quite > as the top 128 bytes is paged and sometimes buggy in terms of access > behaviour. 0x51 contains a bunch of monitoring and other controls > for the module which again can be paged. At 0x56, there may possibly > be some kind of device that translates I2C accesses to MDIO accesses > to access a PHY onboard. > > Consequently, the SFP driver and MDIO translation layer wants access to > the I2C bus, rather than a device. > > Now, before ARM was converted to DT, we had ways to cope with > non-firmware described setups like this by using platform devices and > platform data. Much of that ended up deprecated, because - hey - DT > is great and more modern and the old way is disgusting and we want to > get rid of it. > > However, that approach locks us into describing stuff in firmware, > which is unsuitable when something like this comes along. > > I think what we need is both approaches. We need a way for the SFP > driver (which is a platform_driver) to be used _without_ needing > descriptions in firmware. I think we have that for GPIOs, but for an > I2C bus, We have i2c_get_adapter() for I2C buses, but that needs the > bus number - we could either pass the i2c_adapter or the adapter > number through platform data to the SFP driver. > > Or is there another solution to being able to reuse multi-driver > based infrastructure that we have developed based on DT descriptions > in situations such as an add-in PCI card? The use of software fwnode-s as proposed in this patch-set is another way to deal with this. There has been work to abstract ACPI vs of/dt firmware-nodes into a generic fwnode concept and software-nodes are a third way to define fwnode-s for "struct device" devices. Software nodes currently are mainly used as so called secondary fwnodes which means they can e.g. add extra properties to cover for the firmware description missing some info (which at least on ACPI happens more often then we would like). But a software-node can also be used as the primary fwnode for a device. So what this patch-set does is move the i2c of/dt enumeration code over to the fwnode abstraction (1). This allows the driver for the SPF card to attach a software fwnode to the device for the i2c-controller which describes the hotplug pins + any other always present hw in the same way as it would be done in a devicetree fwnode and then the existing of/dt based SPF code can be re-used as is. At least that is my understanding of this patch-set. Regards, Hans 1) This should result in no functional changes for existing devicetree use cases.
Le Wed, 23 Feb 2022 14:39:27 +0100, Hans de Goede <hdegoede@redhat.com> a écrit : > > I think what we need is both approaches. We need a way for the SFP > > driver (which is a platform_driver) to be used _without_ needing > > descriptions in firmware. I think we have that for GPIOs, but for an > > I2C bus, We have i2c_get_adapter() for I2C buses, but that needs the > > bus number - we could either pass the i2c_adapter or the adapter > > number through platform data to the SFP driver. > > > > Or is there another solution to being able to reuse multi-driver > > based infrastructure that we have developed based on DT descriptions > > in situations such as an add-in PCI card? > > The use of software fwnode-s as proposed in this patch-set is another > way to deal with this. There has been work to abstract ACPI vs > of/dt firmware-nodes into a generic fwnode concept and software-nodes > are a third way to define fwnode-s for "struct device" devices. > > Software nodes currently are mainly used as so called secondary > fwnodes which means they can e.g. add extra properties to cover > for the firmware description missing some info (which at least > on ACPI happens more often then we would like). > > But a software-node can also be used as the primary fwnode for > a device. So what this patch-set does is move the i2c of/dt > enumeration code over to the fwnode abstraction (1). This allows > the driver for the SPF card to attach a software fwnode to the > device for the i2c-controller which describes the hotplug pins + > any other always present hw in the same way as it would be done > in a devicetree fwnode and then the existing of/dt based SPF > code can be re-used as is. > > At least that is my understanding of this patch-set. > > Regards, > > Hans Hello Hans, your understanding is totally correct. This PCIe device actually embeds much more than just a I2C controller. I should have made that clearer in the cover letter, sorry for the confusion. The PCIe card is actually using a lan9662x SoC which is meant to be used as an ethernet switch with 4 ports (2 RJ45 and two SFPS). In order to use this switch, the following drivers can be reused: - lan966x-switch - reset-microchip-sparx5 - lan966x_serdes - reset-microchip-lan966x-phy - mdio-mscc-miim - pinctrl-lan966x - atmel-flexcom - i2c-at91 - i2c-mux - i2c-mux-pinctrl - sfp - clk-lan966x - lan966x-pci-mfd All theses drivers are using of_* API and as such only works with a DT description. One solution that did seems acceptable to me (although not great)was to use mfd_cells and software_node description as primary node. Since I wanted to convert these to be software_node compatible, I had to modify many subsystems (pinctrl, gpio, i2c, clocks, reset, etc). This is why I stated in the cover letter that "This series is part of a larger changeset that touches multiple subsystems". But clearly, it lacks more context and namely the list of subsystems that needed to be modify as well as the PCIe card type. I will modify this cover-letter to add more informations. So indeed, this series is targetting at using devices which uses a software_node as a primary node and modifying subsystems to use the fwnode API in order to make that compatible with these software nodes. As you said, in order to avoid redefining the match tables and allow device_get_match_data to work with software_node, the trick was to reuse the of_table_id However, I'm not totally happy with that as it seems we are doing what was done with the "old" platform_data (a bit cleaner maybe since it allows to reuse the driver with the fwnode API). As Russell asked, I'm also really interested if someone has a solution to reuse device-tree description (overlays ?) to describe such hardware. However, the fact that CONFIG_OF isn't enabled on x86 config seems a bit complicated on this side. This also requires to load a device-tree overlay from the filesystem to describe the card, but that might be something that could be made generic to allow other uses-cases.
On Wed, Feb 23, 2022 at 12:41:47PM +0000, Russell King (Oracle) wrote: > On Wed, Feb 23, 2022 at 01:02:23PM +0100, Hans de Goede wrote: > > On 2/23/22 12:22, Andy Shevchenko wrote: > > > On Tue, Feb 22, 2022 at 02:25:13PM +0100, Clément Léger wrote: > > >> Le Mon, 21 Feb 2022 19:57:39 +0200, > > >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : > > >>> On Mon, Feb 21, 2022 at 05:26:52PM +0100, Clément Léger wrote: > > >>>> Add support to retrieve a i2c bus in sfp with a fwnode. This support > > >>>> is using the fwnode API which also works with device-tree and ACPI. > > >>>> For this purpose, the device-tree and ACPI code handling the i2c > > >>>> adapter retrieval was factorized with the new code. This also allows > > >>>> i2c devices using a software_node description to be used by sfp code. > > >>> > > >>> If I'm not mistaken this patch can even go separately right now, since all used > > >>> APIs are already available. > > >> > > >> This patches uses fwnode_find_i2c_adapter_by_node() which is introduced > > >> by "i2c: fwnode: add fwnode_find_i2c_adapter_by_node()" but they can > > >> probably be contributed both in a separate series. > > > > > > I summon Hans into the discussion since I remember he recently refactored > > > a bit I2C (ACPI/fwnode) APIs. Also he might have an idea about entire big > > > picture approach with this series based on his ACPI experience. > > > > If I understand this series correctly then this is about a PCI-E card > > which has an I2C controller on the card and behind that I2C-controller > > there are a couple if I2C muxes + I2C clients. > > That is what I gathered as well. > > > Assuming I did understand the above correctly. One alternative would be > > to simply manually instantiate the I2C muxes + clients using > > i2c_new_client_device(). But I'm not sure if i2c_new_client_device() > > will work for the muxes without adding some software_nodes which > > brings us back to something like this patch-set. > > That assumes that an I2C device is always present, which is not always > the case - there are hot-pluggable devices on I2C buses. > > Specifically, this series includes pluggable SFP modules, which fall > into this category of "hot-pluggable I2C devices" - spanning several > bus addresses (0x50, 0x51, 0x56). 0x50 is EEPROM like, but not quite > as the top 128 bytes is paged and sometimes buggy in terms of access > behaviour. 0x51 contains a bunch of monitoring and other controls > for the module which again can be paged. At 0x56, there may possibly > be some kind of device that translates I2C accesses to MDIO accesses > to access a PHY onboard. > > Consequently, the SFP driver and MDIO translation layer wants access to > the I2C bus, rather than a device. > > Now, before ARM was converted to DT, we had ways to cope with > non-firmware described setups like this by using platform devices and > platform data. Much of that ended up deprecated, because - hey - DT > is great and more modern and the old way is disgusting and we want to > get rid of it. > > However, that approach locks us into describing stuff in firmware, > which is unsuitable when something like this comes along. Looks like this is a way to reinvent what FPGA should cope with already. And if I remember correctly the discussions about PCIe FPGAs (from 2016, though) the idea is that FPGA should have provided a firmware description with itself. I.o.w. If we are talking about "run-time configurable" devices they should provide a way to bring their description to the system. The currently available way to do it is to get this from EEPROM / ROM specified on the hardware side in form of DT and ACPI blobs (representing overlays). Then the only part that is missed (at least for ACPI case) is to dynamically insert that based on the PCI BDF of the corresponding PCI bridge. TL;DR: In my opinion such hardware must bring the description with itself in case it uses non-enumerable busses, such as SPI, I²C. I dunno what was the last development in this area for FPGAs cases.
> As Russell asked, I'm also really interested if someone has a solution > to reuse device-tree description (overlays ?) to describe such > hardware. However, the fact that CONFIG_OF isn't enabled on x86 config > seems a bit complicated on this side. It does work, intel even used it for one of there tiny x86 SoCs. Maybe it was Newton? If you search around you can find maybe a Linux Plumbers presentation about DT and x86. You can probably use a udev rule, triggered by the PCIe device ID to load the DT overlay. Do you actually need anything from the host other than PCIe? It sounds like this card is pretty self contained, so you won't need phandles pointing to the host i2c bus, or the hosts GPIOs? You only need phandles to your own i2c bus, your own GPIOs? That will make the overlay much simpler. Andrew
Hi, On 2/23/22 16:23, Andrew Lunn wrote: >> As Russell asked, I'm also really interested if someone has a solution >> to reuse device-tree description (overlays ?) to describe such >> hardware. However, the fact that CONFIG_OF isn't enabled on x86 config >> seems a bit complicated on this side. > > It does work, intel even used it for one of there tiny x86 SoCs. Maybe > it was Newton? IIRC those SoCs did not use standard EFI/ACPI though, but rather some other special firmware, I think it was SFI ? This is not so much about the CPU architecture as it is about the firmware/bootloader <-> OS interface. Note I'm not saying this can not be done with EFI/ACPI systems, but I think it has never been tried. Regards, Hans
Hi, On 2/23/22 16:23, Andrew Lunn wrote: >> As Russell asked, I'm also really interested if someone has a solution >> to reuse device-tree description (overlays ?) to describe such >> hardware. However, the fact that CONFIG_OF isn't enabled on x86 config >> seems a bit complicated on this side. > > It does work, intel even used it for one of there tiny x86 SoCs. Maybe > it was Newton? IIRC those SoCs did not use standard EFI/ACPI though, but rather some other special firmware, I think it was SFI ? This is not so much about the CPU architecture as it is about the firmware/bootloader <-> OS interface. Note I'm not saying this can not be done with EFI/ACPI systems, but I think it has never been tried. Regards, Hans
On Wed, Feb 23, 2022 at 04:27:33PM +0100, Hans de Goede wrote: > On 2/23/22 16:23, Andrew Lunn wrote: > >> As Russell asked, I'm also really interested if someone has a solution > >> to reuse device-tree description (overlays ?) to describe such > >> hardware. However, the fact that CONFIG_OF isn't enabled on x86 config > >> seems a bit complicated on this side. > > > > It does work, intel even used it for one of there tiny x86 SoCs. Maybe > > it was Newton? > > IIRC those SoCs did not use standard EFI/ACPI though, but rather some > other special firmware, I think it was SFI ? This is not so much about > the CPU architecture as it is about the firmware/bootloader <-> > OS interface. I think Andrew refers to Intel SoCs that are using OF. Those so far are CE4xxx and SoFIA SoCs.
Le Wed, 23 Feb 2022 16:23:46 +0100, Andrew Lunn <andrew@lunn.ch> a écrit : > > As Russell asked, I'm also really interested if someone has a solution > > to reuse device-tree description (overlays ?) to describe such > > hardware. However, the fact that CONFIG_OF isn't enabled on x86 config > > seems a bit complicated on this side. > > It does work, intel even used it for one of there tiny x86 SoCs. Maybe > it was Newton? If you search around you can find maybe a Linux > Plumbers presentation about DT and x86. Oh yes, I know it works and I tried loading an overlay using CONFIG_OF on a x86. Currently it does not works and generate a oops due to the fact that the lack of "/" node is not handled and that the error path has probably not been thoroughly tested. Adress remapping for PCI and lack of PCI bus description might also be a bit cumbersome but maybe this is the way to go. I was saying a "bit complicated" as it is not often used and it would require enabling CONFIG_OF to support this feature as well as allowing loading overlays without any root node. But this is probably something that is also achievable. > > You can probably use a udev rule, triggered by the PCIe device ID to > load the DT overlay. Or maybe load the overlay from the PCIe driver ? This would also allow to introduce some remapping of addresses (see below) inside the driver maybe. > > Do you actually need anything from the host other than PCIe? It sounds > like this card is pretty self contained, so you won't need phandles > pointing to the host i2c bus, or the hosts GPIOs? You only need > phandles to your own i2c bus, your own GPIOs? That will make the > overlay much simpler. Yes, the device is almost self contained, only the IRQ needs to be chained with the MSI. > > Andrew
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index 4720b24ca51b..9d9e3d209408 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -2499,43 +2499,25 @@ static int sfp_probe(struct platform_device *pdev) return err; sff = sfp->type = &sfp_data; + if (dev_fwnode(&pdev->dev)) { + struct fwnode_handle *fwnode = dev_fwnode(&pdev->dev); + struct fwnode_handle *np; - if (pdev->dev.of_node) { - struct device_node *node = pdev->dev.of_node; - const struct of_device_id *id; - struct device_node *np; + if (!is_acpi_device_node(fwnode)) { + sff = device_get_match_data(&pdev->dev); + if (WARN_ON(!sff)) + return -EINVAL; - id = of_match_node(sfp_of_match, node); - if (WARN_ON(!id)) - return -EINVAL; - - sff = sfp->type = id->data; - - np = of_parse_phandle(node, "i2c-bus", 0); - if (!np) { - dev_err(sfp->dev, "missing 'i2c-bus' property\n"); - return -ENODEV; + sfp->type = sff; } - i2c = of_find_i2c_adapter_by_node(np); - of_node_put(np); - } else if (has_acpi_companion(&pdev->dev)) { - struct acpi_device *adev = ACPI_COMPANION(&pdev->dev); - struct fwnode_handle *fw = acpi_fwnode_handle(adev); - struct fwnode_reference_args args; - struct acpi_handle *acpi_handle; - int ret; - - ret = acpi_node_get_property_reference(fw, "i2c-bus", 0, &args); - if (ret || !is_acpi_device_node(args.fwnode)) { - dev_err(&pdev->dev, "missing 'i2c-bus' property\n"); + np = fwnode_find_reference(fwnode, "i2c-bus", 0); + if (!np) { + dev_err(&pdev->dev, "Cannot parse i2c-bus\n"); return -ENODEV; } - - acpi_handle = ACPI_HANDLE_FWNODE(args.fwnode); - i2c = i2c_acpi_find_adapter_by_handle(acpi_handle); - } else { - return -EINVAL; + i2c = fwnode_find_i2c_adapter_by_node(np); + fwnode_handle_put(np); } if (!i2c)
Add support to retrieve a i2c bus in sfp with a fwnode. This support is using the fwnode API which also works with device-tree and ACPI. For this purpose, the device-tree and ACPI code handling the i2c adapter retrieval was factorized with the new code. This also allows i2c devices using a software_node description to be used by sfp code. Signed-off-by: Clément Léger <clement.leger@bootlin.com> --- drivers/net/phy/sfp.c | 44 +++++++++++++------------------------------ 1 file changed, 13 insertions(+), 31 deletions(-)