linux-kernel - Re: [PATCH 1/3] dt-bindings: net: can: m_can: Add Documentation for stb-gpios

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:   Mon, 3 Feb 2020 12:06:10 +0000
From:   Rob Herring <robh@...nel.org>
To:     Faiz Abbas <faiz_abbas@...com>
Cc:     Dan Murphy <dmurphy@...com>, Sekhar Nori <nsekhar@...com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, netdev@...r.kernel.org,
        linux-can@...r.kernel.org, catalin.marinas@....com,
        mark.rutland@....com, davem@...emloft.net, mkl@...gutronix.de,
        wg@...ndegger.com, sriram.dash@...sung.com, nm@...com,
        t-kristo@...com
Subject: Re: [PATCH 1/3] dt-bindings: net: can: m_can: Add Documentation for
 stb-gpios

On Thu, Jan 23, 2020 at 01:09:41PM +0530, Faiz Abbas wrote:
> Hi,
> 
> On 22/01/20 8:04 pm, Dan Murphy wrote:
> > Sekhar
> > 
> > On 1/22/20 8:24 AM, Sekhar Nori wrote:
> >> On 22/01/20 7:05 PM, Dan Murphy wrote:
> >>> Faiz
> >>>
> >>> On 1/22/20 2:03 AM, Faiz Abbas wrote:
> >>>> The CAN transceiver on some boards has an STB pin which is
> >>>> used to control its standby mode. Add an optional property
> >>>> stb-gpios to toggle the same.
> >>>>
> >>>> Signed-off-by: Faiz Abbas <faiz_abbas@...com>
> >>>> Signed-off-by: Sekhar Nori <nsekhar@...com>
> >>>> ---
> >>>>    Documentation/devicetree/bindings/net/can/m_can.txt | 2 ++
> >>>>    1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt
> >>>> b/Documentation/devicetree/bindings/net/can/m_can.txt
> >>>> index ed614383af9c..cc8ba3f7a2aa 100644
> >>>> --- a/Documentation/devicetree/bindings/net/can/m_can.txt
> >>>> +++ b/Documentation/devicetree/bindings/net/can/m_can.txt
> >>>> @@ -48,6 +48,8 @@ Optional Subnode:
> >>>>                  that can be used for CAN/CAN-FD modes. See
> >>>>                
> >>>> Documentation/devicetree/bindings/net/can/can-transceiver.txt
> >>>>                  for details.
> >>>> +stb-gpios        : gpio node to toggle the STB (standby) signal on
> >>>> the transceiver
> >>>> +
> >>> The m_can.txt is for the m_can framework.  If this is specific to the
> >>> platform then it really does not belong here.
> >>>
> >>> If the platform has specific nodes then maybe we need a
> >>> m_can_platform.txt binding for specific platform nodes.  But I leave
> >>> that decision to Rob.
> >> Since this is transceiver enable, should this not be in
> >> Documentation/devicetree/bindings/net/can/can-transceiver.txt?
> > 
> 
> The transceiver node is just a node without an associated device. I had
> tried to convert it to a phy implementation but that idea got shot down
> here:
> 
> https://lore.kernel.org/patchwork/patch/1006238/

Nodes and drivers are not a 1-1 thing. Is the transceiver a separate h/w 
device? If so, then it should be a separate node and properties of that 
device go in its node. Also, nothing is stopping you from using the PHY 
binding without using the kernel's PHY framework.

As to whether it should be a separate phy driver, I think probably the 
wrong decision was made. We always seem to start out with no PHY on 
these things and the complexity just grows until we need one. 

Rob