lkml.org 
[lkml]   [2022]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC 00/10] add support for fwnode in i2c mux system and sfp
    On Tue, Feb 22, 2022 at 05:30:19PM +0100, Clément Léger wrote:
    > Le Mon, 21 Feb 2022 19:41:24 +0200,
    > Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit :

    > > > We thought about adding CONFIG_OF to x86 and potentially describe this
    > > > card using device-tree overlays but it introduce other problems that
    > > > also seems difficult to solve (overlay loading without base
    > > > device-tree, fixup of IRQs, adresses, and so on) and CONFIG_OF is not
    > > > often enabled on x86 to say the least.
    > >
    > > Why it can't be described by SSDT overlay (if the x86 platform in question is
    > > ACPI based)?
    >
    > This devices uses a SoC for which drivers are already available but are
    > meant to be used by a device-tree description. These drivers uses the
    > following subsystems:
    > - reset (no ACPI support ?)
    > - clk (no ACPI support ?)
    > - pinctrl (no ACPI support ?)
    > - syscon (no ACPI support ?)
    > - gpio
    > - phy
    > - mdio
    >
    > Converting existing OF support to fwnode support and thus allowing
    > drivers and subsystems to be compatible with software nodes seemed like
    > the easiest way to do what I needed by keeping all existing drivers.
    > With this support, the driver is completely self-contained and does
    > allow the card to be plugged on whatever platform the user may have.

    I agree with Hans on the point that converting to / supporting fwnode is
    a good thing by its own.

    > Again, the PCI card is independent of the platform, I do not really see
    > why it should be described using platform description language.

    Yep, and that why it should cope with the platforms it's designed to be used
    with.

    > > > This series introduce a number of changes in multiple subsystems to
    > > > allow registering and using devices that are described with a
    > > > software_node description attached to a mfd_cell, making them usable
    > > > with the fwnode API. It was needed to modify many subsystem where
    > > > CONFIG_OF was tightly integrated through the use of of_xlate()
    > > > functions and other of_* calls. New calls have been added to use fwnode
    > > > API and thus be usable with a wider range of nodes. Functions that are
    > > > used to get the devices (pinctrl_get, clk_get and so on) also needed
    > > > to be changed to use the fwnode API internally.
    > > >
    > > > For instance, the clk framework has been modified to add a
    > > > fwnode_xlate() callback and a new named fwnode_clk_add_hw_provider()
    > > > has been added. This function will register a clock using
    > > > fwnode_xlate() callback. Note that since the fwnode API is compatible
    > > > with devices that have a of_node member set, it will still be possible
    > > > to use the driver and get the clocks with CONFIG_OF enabled
    > > > configurations.
    > >
    > > How does this all is compatible with ACPI approaches?
    > > I mean we usually do not reintroduce 1:1 DT schemas in ACPI.
    >
    > For the moment, I only added fwnode API support as an alternative to
    > support both OF and software nodes. ACPI is not meant to be handled by
    > this code "as-is". There is for sure some modifications to be made and
    > I do not know how clocks are handled when using ACPI. Based on some
    > thread dating back to 2018 [1], it seem it was even not supported at
    > all.
    >
    > To be clear, I added the equivalent of the OF support but using
    > fwnode API because I was interested primarly in using it with software
    > nodes and still wanted OF support to work. I did not planned it to be
    > "ACPI compliant" right now since I do not have any knowledge in that
    > field.

    And here is the problem. We have a few different resource providers
    (a.k.a. firmware interfaces) which we need to cope with.

    What is going on in this series seems to me quite a violation of the
    layers and technologies. But I guess you may find a supporter of your
    ideas (I mean Enrico). However, I'm on the other side and do not like
    this approach.

    > > I think the CCF should be converted to use fwnode APIs and meanwhile
    > > we may discuss how to deal with clocks on ACPI platforms, because
    > > it may be a part of the power management methods.
    >
    > Ok, before going down that way, should the fwnode support be the "only"
    > one, ie remove of_clk_register and others and convert them to
    > fwnode_clk_register for instance or should it be left to avoid
    > modifying all clock drivers ?

    IRQ domain framework decided to cohabit both, while deprecating the OF one.
    (see "add" vs. "create" APIs there). I think it's a sane choice.

    > > > In some subsystems, it was possible to keep OF related function by
    > > > wrapping the fwnode ones. It is not yet sure if both support
    > > > (device-tree and fwnode) should still continue to coexists. For instance
    > > > if fwnode_xlate() and of_xlate() should remain since the fwnode version
    > > > also supports device-tree. Removing of_xlate() would of course require
    > > > to modify all drivers that uses it.
    > > >
    > > > Here is an excerpt of the lan966x description when used as a PCIe card.
    > > > The complete description is visible at [2]. This part only describe the
    > > > flexcom controller and the fixed-clock that is used as an input clock.
    > > >
    > > > static const struct property_entry ddr_clk_props[] = {
    > > > PROPERTY_ENTRY_U32("clock-frequency", 30000000),
    > >
    > > > PROPERTY_ENTRY_U32("#clock-cells", 0),
    > >
    > > Why this is used?
    >
    > These props actually describes a fixed-clock properties. When adding
    > fwnode support to clk framework, it was needed to add the
    > equivalent of of_xlate() for fwnode (fwnode_xlate()). The number of
    > cells used to describe a reference is still needed to do the
    > translation using fwnode_property_get_reference_args() and give the
    > correct arguments to fwnode_xlate().

    What you described is the programming (overkilled) point. But does hardware
    needs this? I.o.w. does it make sense in the _hardware_ description?

    > [1]
    > https://lore.kernel.org/lkml/914341e7-ca94-054d-6127-522b745006b4@arm.com/T/

    --
    With Best Regards,
    Andy Shevchenko


    \
     
     \ /
      Last update: 2022-02-23 15:48    [W:3.087 / U:0.364 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site