Re: [PATCH 1/2] dt-bindings: phy: ti,tcan104x-can: Document Microchip ATA6561

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3417 bytes --]

On Tue, Jul 23, 2024 at 08:20:04PM +0300, IlorDash wrote:
> On Fri, 19 Jul 2024 at 18:07, Conor Dooley <conor@kernel.org> wrote:
> >
> > On Fri, Jul 19, 2024 at 12:03:21AM +0300, Ilya Orazov wrote:
> > > Microchip ATA6561 is High-Speed CAN Transceiver with Standby Mode.
> > > It is pin-compatible with TI TCAN1042.
> > >
> > > Signed-off-by: Ilya Orazov <ilordash02@gmail.com>
> > > ---
> > >  Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
> > > index 79dad3e89aa6..03de361849d2 100644
> > > --- a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
> > > +++ b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
> > > @@ -18,6 +18,7 @@ properties:
> > >        - nxp,tjr1443
> > >        - ti,tcan1042
> > >        - ti,tcan1043
> > > +      - microchip,ata6561
> >
> > Given that your driver patch has
> > | diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
> > | index ee4ce4249698..dbcd99213ba1 100644
> > | --- a/drivers/phy/phy-can-transceiver.c
> > | +++ b/drivers/phy/phy-can-transceiver.c
> > | @@ -89,6 +89,10 @@ static const struct of_device_id can_transceiver_phy_ids[] = {
> > |                 .compatible = "nxp,tjr1443",
> > |                 .data = &tcan1043_drvdata
> > |         },
> > | +       {
> > | +               .compatible = "microchip,ata6561",
> > | +               .data = &tcan1042_drvdata
> > | +       },
> > |         { }
> > |  };
> >
> > the driver patch is actually not needed at all, and you just need to
> > allow ti,tcan1042 as fallback compatible in the binding, so something
> > like:
> >
> >   compatible:
> >     oneOf:
> >       - enum:
> >           - nxp,tjr1443
> >           - ti,tcan1042
> >           - ti,tcan1043
> >       - items:
> >           - const: microchip,ata6561
> >           - const: ti,tcan1042
> >
> >    '#phy-cells':
> >      const: 0
> 
> I tested the build with fallback compatible:
> 
> compatible:
>   oneOf:
>     - items:
>       - enum:
>         - microchip,ata6561
>       - const: ti,tcan1042
>     - items:
>       - enum:
>         - nxp,tjr1443
>       - const: ti,tcan1043
> 
> and modified compatible property in DTS:
> 
> compatible = "microchip,ata6561", "ti,tcan1042";
> 
> Build succeeded, phy-can-transceiver driver was used. So I would like
> to add a fallback compatible for both "microchip,ata6561" and
> "nxp,tjr1443" in this binding and modify other DTS files with
> compatible = "nxp,tjr1443". What do you think?

This is wrong on two counts. Firstly, were what you have correct, you
should
squash the two:
     - items:
         - enum:
           - nxp,tjr1443
           - microchip,ata6561
         - const: ti,tcan1042

However, that does not allow the TI compatibles in isolation, so you
still need to allow that for the actual TI devices, so you need:

   oneOf:
     - items:
         - enum:
           - microchip,ata6561
           - nxp,tjr1443
           - ti,tcan1043
         - const: ti,tcan1042
     - const: ti,tcan1042

There's probably some devicetrees that would need to be fixed up. I'm
just not convinced that this is worth retrofitting however.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy