linux-kernel - Re: [PATCH 06/12] net: phy: add phy_device_atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 Apr 2023 22:34:24 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Marco Felsch <m.felsch@...gutronix.de>
Cc:     Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@...adcom.com>,
        Richard Cochran <richardcochran@...il.com>,
        Radu Pirea <radu-nicolae.pirea@....nxp.com>,
        Shyam Sundar S K <Shyam-sundar.S-k@....com>,
        Yisen Zhuang <yisen.zhuang@...wei.com>,
        Salil Mehta <salil.mehta@...wei.com>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Iyappan Subramanian <iyappan@...amperecomputing.com>,
        Keyur Chudgar <keyur@...amperecomputing.com>,
        Quan Nguyen <quan@...amperecomputing.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>, Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
        devicetree@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH 06/12] net: phy: add phy_device_atomic_register helper

> Currently we have one API which creates/allocates the 'struct
> phy_device' and intialize the state which is:
>    - phy_device_create()
> 
> This function requests a driver based on the phy_id/c45_ids. The ID have
> to come from somewhere if autodection is used. For autodetection case
>    - get_phy_device()
> 
> is called. This function try to access the phy without taken possible
> hardware dependencies into account. These dependecies can be reset-lines
> (in my case), clocks, supplies, ...
> 
> For taking fwnode (and possible dependencies) into account fwnode_mdio.c
> was written which provides two helpers:
>    - fwnode_mdiobus_register_phy()
>    - fwnode_mdiobus_phy_device_register().
> 
> The of_mdio.c and of_mdiobus_register_phy() is just a wrapper around
> fwnode_mdiobus_register_phy().

It seems to me that the real problem is that mdio_device_reset() takes
an mdio_device. mdiobus_register_gpiod() and mdiobus_register_reset()
also take an mdio_device. These are the functions you want to call
before calling of_mdiobus_register_phy() in __of_mdiobus_register() to
ensure the PHY is out of reset. But you don't have an mdio_device yet.

So i think a better solution is to refactor this code. Move the
resources into a structure of their own, and make that a member of
mdio_device. You can create a stack version of this resource structure
in __of_mdiobus_register(), parse DT to fill it out by calling
mdiobus_register_gpiod() and mdiobus_register_reset() taking this new
structure, take it out of reset by calling mdio_device_reset(), and
then call of_mdiobus_register_phy(). If a PHY is found, copy the
values in the resulting mdio_device. If not, release the resources.

Doing it like this means there is no API change.

      Andrew