[PATCH 0/2] phy: Add support for Microchip ATA6561 CAN Transceiver

[PATCH 0/2] phy: Add support for Microchip ATA6561 CAN Transceiver

[Ilya Orazov]

Hi all,

The Microchip ATA6561 is a High-Speed CAN Transceiver with Standby Mode,
and it is pin-compatible with TI TCAN1042. Therefore, this patch series
extends support for the TI TCAN1042 DT bindings and the generic
CAN Transceiver PHY driver.

This CAN transceiver is a popular chip that I have used in my boards.
I decided to add support for ATA6561 to the kernel, as I believe it would be
beneficial.

Thank you for your feedback!

Ilya Orazov (2):
  dt-bindings: phy: ti,tcan104x-can: Document Microchip ATA6561
  phy: phy-can-transceiver: Add support for Microchip ATA6561

 Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml | 1 +
 drivers/phy/phy-can-transceiver.c                          | 4 ++++
 2 files changed, 5 insertions(+)

-- 
2.34.1

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

[Ilya Orazov]

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
 
   '#phy-cells':
     const: 0
-- 
2.34.1

[PATCH 2/2] phy: phy-can-transceiver: Add support for Microchip ATA6561

[Ilya Orazov]

The 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>
---
 drivers/phy/phy-can-transceiver.c | 4 ++++
 1 file changed, 4 insertions(+)

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
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, can_transceiver_phy_ids);
-- 
2.34.1

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

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

Cheers,
Conor.

>  
>    '#phy-cells':
>      const: 0
> -- 
> 2.34.1
> 
> 

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

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

[IlorDash]

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
>
> Cheers,
> Conor.
>
> >
> >    '#phy-cells':
> >      const: 0
> > --
> > 2.34.1
> >
> >

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?

-- 
Best regards,
Ilya Orazov

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

[Conor Dooley]

[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Ilya Orazov]

On Tue, 23 Jul 2024 at 21:50, Conor Dooley <conor@kernel.org> wrote:
>
> 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.

But nxp,tjr1443 is pin compatible with ti,tcan1043, so it should
fallback only to ti,tcan1043 and not ti,tcan1042. That's why I decided
to split them into different enums.

I made my patch according to a similar one that adds support for
nxp,tjr1443. You can find it's conversation on
https://lore.kernel.org/all/6ee5e2ce00019bd3f77d6a702b38bab1a45f3bb0.1674037830.git.geert+renesas@glider.be/t/#u.
I thought we want to hold all PHY chip names in one compatible enum
and each in its own of_device_id struct in driver and extend them
where appropriate.

-- 
Best regards,
Ilya Orazov

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5420 bytes --]

On Tue, Jul 23, 2024 at 10:55:17PM +0300, Ilya Orazov wrote:
> On Tue, 23 Jul 2024 at 21:50, Conor Dooley <conor@kernel.org> wrote:
> >
> > 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.
> 
> But nxp,tjr1443 is pin compatible with ti,tcan1043, so it should
> fallback only to ti,tcan1043 and not ti,tcan1042. That's why I decided
> to split them into different enums.

Ah, sorry I missed that. I misread the match data. Then you need:
  compatible:
    oneOf:
      - items:
        - enum:
          - microchip,ata6561
        - const: ti,tcan1042
      - items:
        - enum:
          - nxp,tjr1443
        - const: ti,tcan1043
      - enum:
          const: ti,tcan1042
          const: ti,tcan1043

because the TI devices exist and we still need to be able to
differentiate the TI and NXP devices. If you have
  compatible = "nxp,tjr1443", "ti,tcan1042";
that means the device is an nxp,tjr1443. If you have
  compatible = "ti,tcan1042";
then that's a tcan1042.

> I made my patch according to a similar one that adds support for
> nxp,tjr1443. You can find it's conversation on
> https://lore.kernel.org/all/6ee5e2ce00019bd3f77d6a702b38bab1a45f3bb0.1674037830.git.geert+renesas@glider.be/t/#u.

> I thought we want to hold all PHY chip names in one compatible enum
> and each in its own of_device_id struct in driver and extend them
> where appropriate.

Nah, fallbacks are preferred when the programming model is either
identical or a "compatible superset" of an existing device. New
of_device_id structs should only be used where we need to account for
differences in the programming model.

Cheers,
Conor.

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

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

[Ilya Orazov]

Thank you for your detailed explanation. I support the decision to use
fallback compatible.

However, I am curious as to why the NXP CAN PHY transceiver was not
included as fallback compatible. Geert, could you please share your
thoughts on this matter?

On Tue, 23 Jul 2024 at 23:14, Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Jul 23, 2024 at 10:55:17PM +0300, Ilya Orazov wrote:
> > On Tue, 23 Jul 2024 at 21:50, Conor Dooley <conor@kernel.org> wrote:
> > >
> > > 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.
> >
> > But nxp,tjr1443 is pin compatible with ti,tcan1043, so it should
> > fallback only to ti,tcan1043 and not ti,tcan1042. That's why I decided
> > to split them into different enums.
>
> Ah, sorry I missed that. I misread the match data. Then you need:
>   compatible:
>     oneOf:
>       - items:
>         - enum:
>           - microchip,ata6561
>         - const: ti,tcan1042
>       - items:
>         - enum:
>           - nxp,tjr1443
>         - const: ti,tcan1043
>       - enum:
>           const: ti,tcan1042
>           const: ti,tcan1043
>
> because the TI devices exist and we still need to be able to
> differentiate the TI and NXP devices. If you have
>   compatible = "nxp,tjr1443", "ti,tcan1042";
> that means the device is an nxp,tjr1443. If you have
>   compatible = "ti,tcan1042";
> then that's a tcan1042.
>
> > I made my patch according to a similar one that adds support for
> > nxp,tjr1443. You can find it's conversation on
> > https://lore.kernel.org/all/6ee5e2ce00019bd3f77d6a702b38bab1a45f3bb0.1674037830.git.geert+renesas@glider.be/t/#u.
>
> > I thought we want to hold all PHY chip names in one compatible enum
> > and each in its own of_device_id struct in driver and extend them
> > where appropriate.
>
> Nah, fallbacks are preferred when the programming model is either
> identical or a "compatible superset" of an existing device. New
> of_device_id structs should only be used where we need to account for
> differences in the programming model.
>
> Cheers,
> Conor.



-- 
Best regards,
Ilya Orazov

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

[Geert Uytterhoeven]

Hi Ilya,

On Sun, Jul 28, 2024 at 10:52 AM Ilya Orazov <ilordash02@gmail.com> wrote:
> On Tue, 23 Jul 2024 at 23:14, Conor Dooley <conor@kernel.org> wrote:
> > On Tue, Jul 23, 2024 at 10:55:17PM +0300, Ilya Orazov wrote:
> > > On Tue, 23 Jul 2024 at 21:50, Conor Dooley <conor@kernel.org> wrote:
> > > > 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.
> > >
> > > But nxp,tjr1443 is pin compatible with ti,tcan1043, so it should
> > > fallback only to ti,tcan1043 and not ti,tcan1042. That's why I decided
> > > to split them into different enums.
> >
> > Ah, sorry I missed that. I misread the match data. Then you need:
> >   compatible:
> >     oneOf:
> >       - items:
> >         - enum:
> >           - microchip,ata6561
> >         - const: ti,tcan1042
> >       - items:
> >         - enum:
> >           - nxp,tjr1443
> >         - const: ti,tcan1043
> >       - enum:
> >           const: ti,tcan1042
> >           const: ti,tcan1043
> >
> > because the TI devices exist and we still need to be able to
> > differentiate the TI and NXP devices. If you have
> >   compatible = "nxp,tjr1443", "ti,tcan1042";
> > that means the device is an nxp,tjr1443. If you have
> >   compatible = "ti,tcan1042";
> > then that's a tcan1042.
> >
> > > I made my patch according to a similar one that adds support for
> > > nxp,tjr1443. You can find it's conversation on
> > > https://lore.kernel.org/all/6ee5e2ce00019bd3f77d6a702b38bab1a45f3bb0.1674037830.git.geert+renesas@glider.be/t/#u.
> >
> > > I thought we want to hold all PHY chip names in one compatible enum
> > > and each in its own of_device_id struct in driver and extend them
> > > where appropriate.
> >
> > Nah, fallbacks are preferred when the programming model is either
> > identical or a "compatible superset" of an existing device. New
> > of_device_id structs should only be used where we need to account for
> > differences in the programming model.
>
> However, I am curious as to why the NXP CAN PHY transceiver was not
> included as fallback compatible. Geert, could you please share your
> thoughts on this matter?

The TJR1443 looked sufficiently similar to the TCAN1043 to use the
same driver configuration (which is limited to having standby and/or
enable signals or not).  However, I'm not sure it behaves exactly
the same, e.g. in case of reporting an error condition (which is not
yet supported by the driver). The part numbers are also different,
so this is not a simple case of SN74HCxx vs. CD74HCxx.

Summary: I don't know if they are identical, or if TJR1443 is a
compatible superset of TCAN1043, or vice versa. Hence I went for the
safest way....

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 7368 bytes --]

On Mon, Jul 29, 2024 at 10:51:50AM +0200, Geert Uytterhoeven wrote:
> Hi Ilya,
> 
> On Sun, Jul 28, 2024 at 10:52 AM Ilya Orazov <ilordash02@gmail.com> wrote:
> > On Tue, 23 Jul 2024 at 23:14, Conor Dooley <conor@kernel.org> wrote:
> > > On Tue, Jul 23, 2024 at 10:55:17PM +0300, Ilya Orazov wrote:
> > > > On Tue, 23 Jul 2024 at 21:50, Conor Dooley <conor@kernel.org> wrote:
> > > > > 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.
> > > >
> > > > But nxp,tjr1443 is pin compatible with ti,tcan1043, so it should
> > > > fallback only to ti,tcan1043 and not ti,tcan1042. That's why I decided
> > > > to split them into different enums.
> > >
> > > Ah, sorry I missed that. I misread the match data. Then you need:
> > >   compatible:
> > >     oneOf:
> > >       - items:
> > >         - enum:
> > >           - microchip,ata6561
> > >         - const: ti,tcan1042
> > >       - items:
> > >         - enum:
> > >           - nxp,tjr1443
> > >         - const: ti,tcan1043
> > >       - enum:
> > >           const: ti,tcan1042
> > >           const: ti,tcan1043
> > >
> > > because the TI devices exist and we still need to be able to
> > > differentiate the TI and NXP devices. If you have
> > >   compatible = "nxp,tjr1443", "ti,tcan1042";
> > > that means the device is an nxp,tjr1443. If you have
> > >   compatible = "ti,tcan1042";
> > > then that's a tcan1042.
> > >
> > > > I made my patch according to a similar one that adds support for
> > > > nxp,tjr1443. You can find it's conversation on
> > > > https://lore.kernel.org/all/6ee5e2ce00019bd3f77d6a702b38bab1a45f3bb0.1674037830.git.geert+renesas@glider.be/t/#u.
> > >
> > > > I thought we want to hold all PHY chip names in one compatible enum
> > > > and each in its own of_device_id struct in driver and extend them
> > > > where appropriate.
> > >
> > > Nah, fallbacks are preferred when the programming model is either
> > > identical or a "compatible superset" of an existing device. New
> > > of_device_id structs should only be used where we need to account for
> > > differences in the programming model.
> >
> > However, I am curious as to why the NXP CAN PHY transceiver was not
> > included as fallback compatible. Geert, could you please share your
> > thoughts on this matter?
> 
> The TJR1443 looked sufficiently similar to the TCAN1043 to use the
> same driver configuration (which is limited to having standby and/or
> enable signals or not).  However, I'm not sure it behaves exactly
> the same, e.g. in case of reporting an error condition (which is not
> yet supported by the driver). The part numbers are also different,
> so this is not a simple case of SN74HCxx vs. CD74HCxx.
> 
> Summary: I don't know if they are identical, or if TJR1443 is a
> compatible superset of TCAN1043, or vice versa. Hence I went for the
> safest way....

If we don't know for sure what the craic is with compatibility, then we
should leave the existing tjr1443 compatible as-is I think.

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

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

[Ilya Orazov]

On Thu, 1 Aug 2024 at 18:12, Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Jul 29, 2024 at 10:51:50AM +0200, Geert Uytterhoeven wrote:
> > Hi Ilya,
> >
> > On Sun, Jul 28, 2024 at 10:52 AM Ilya Orazov <ilordash02@gmail.com> wrote:
> > > On Tue, 23 Jul 2024 at 23:14, Conor Dooley <conor@kernel.org> wrote:
> > > > On Tue, Jul 23, 2024 at 10:55:17PM +0300, Ilya Orazov wrote:
> > > > > On Tue, 23 Jul 2024 at 21:50, Conor Dooley <conor@kernel.org> wrote:
> > > > > > 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.
> > > > >
> > > > > But nxp,tjr1443 is pin compatible with ti,tcan1043, so it should
> > > > > fallback only to ti,tcan1043 and not ti,tcan1042. That's why I decided
> > > > > to split them into different enums.
> > > >
> > > > Ah, sorry I missed that. I misread the match data. Then you need:
> > > >   compatible:
> > > >     oneOf:
> > > >       - items:
> > > >         - enum:
> > > >           - microchip,ata6561
> > > >         - const: ti,tcan1042
> > > >       - items:
> > > >         - enum:
> > > >           - nxp,tjr1443
> > > >         - const: ti,tcan1043
> > > >       - enum:
> > > >           const: ti,tcan1042
> > > >           const: ti,tcan1043
> > > >
> > > > because the TI devices exist and we still need to be able to
> > > > differentiate the TI and NXP devices. If you have
> > > >   compatible = "nxp,tjr1443", "ti,tcan1042";
> > > > that means the device is an nxp,tjr1443. If you have
> > > >   compatible = "ti,tcan1042";
> > > > then that's a tcan1042.
> > > >
> > > > > I made my patch according to a similar one that adds support for
> > > > > nxp,tjr1443. You can find it's conversation on
> > > > > https://lore.kernel.org/all/6ee5e2ce00019bd3f77d6a702b38bab1a45f3bb0.1674037830.git.geert+renesas@glider.be/t/#u.
> > > >
> > > > > I thought we want to hold all PHY chip names in one compatible enum
> > > > > and each in its own of_device_id struct in driver and extend them
> > > > > where appropriate.
> > > >
> > > > Nah, fallbacks are preferred when the programming model is either
> > > > identical or a "compatible superset" of an existing device. New
> > > > of_device_id structs should only be used where we need to account for
> > > > differences in the programming model.
> > >
> > > However, I am curious as to why the NXP CAN PHY transceiver was not
> > > included as fallback compatible. Geert, could you please share your
> > > thoughts on this matter?
> >
> > The TJR1443 looked sufficiently similar to the TCAN1043 to use the
> > same driver configuration (which is limited to having standby and/or
> > enable signals or not).  However, I'm not sure it behaves exactly
> > the same, e.g. in case of reporting an error condition (which is not
> > yet supported by the driver). The part numbers are also different,
> > so this is not a simple case of SN74HCxx vs. CD74HCxx.
> >
> > Summary: I don't know if they are identical, or if TJR1443 is a
> > compatible superset of TCAN1043, or vice versa. Hence I went for the
> > safest way....
>
> If we don't know for sure what the craic is with compatibility, then we
> should leave the existing tjr1443 compatible as-is I think.

If I understood the kernel documentation correctly, we use fallback
compatibles when devices are similar or there is an iterative
relationship between them. In my case, the TCAN1042 and ATA6561 are
from different manufacturers, and I'm not sure about their fully
identical functionality.

Therefore, I'll go back to the original idea where I shouldn't use a
fallback compatible here and must leave it as another compatible
property with its own of_device_id struct.

What do you think about it? In my opinion, this is not a case for
fallback compatibility.

-- 
Best regards,
Ilya Orazov

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]

On Sat, Aug 03, 2024 at 03:31:52PM +0300, Ilya Orazov wrote:
> > > > > > I made my patch according to a similar one that adds support for
> > > > > > nxp,tjr1443. You can find it's conversation on
> > > > > > https://lore.kernel.org/all/6ee5e2ce00019bd3f77d6a702b38bab1a45f3bb0.1674037830.git.geert+renesas@glider.be/t/#u.
> > > > >
> > > > > > I thought we want to hold all PHY chip names in one compatible enum
> > > > > > and each in its own of_device_id struct in driver and extend them
> > > > > > where appropriate.
> > > > >
> > > > > Nah, fallbacks are preferred when the programming model is either
> > > > > identical or a "compatible superset" of an existing device. New
> > > > > of_device_id structs should only be used where we need to account for
> > > > > differences in the programming model.
> > > >
> > > > However, I am curious as to why the NXP CAN PHY transceiver was not
> > > > included as fallback compatible. Geert, could you please share your
> > > > thoughts on this matter?
> > >
> > > The TJR1443 looked sufficiently similar to the TCAN1043 to use the
> > > same driver configuration (which is limited to having standby and/or
> > > enable signals or not).  However, I'm not sure it behaves exactly
> > > the same, e.g. in case of reporting an error condition (which is not
> > > yet supported by the driver). The part numbers are also different,
> > > so this is not a simple case of SN74HCxx vs. CD74HCxx.
> > >
> > > Summary: I don't know if they are identical, or if TJR1443 is a
> > > compatible superset of TCAN1043, or vice versa. Hence I went for the
> > > safest way....
> >
> > If we don't know for sure what the craic is with compatibility, then we
> > should leave the existing tjr1443 compatible as-is I think.
> 
> If I understood the kernel documentation correctly, we use fallback
> compatibles when devices are similar or there is an iterative
> relationship between them. In my case, the TCAN1042 and ATA6561 are
> from different manufacturers, and I'm not sure about their fully
> identical functionality.

It's about programming models being compatible, not identical. The
manufacturer also doesn't matter.

> Therefore, I'll go back to the original idea where I shouldn't use a
> fallback compatible here and must leave it as another compatible
> property with its own of_device_id struct.
> 
> What do you think about it? In my opinion, this is not a case for
> fallback compatibility.

Why is it not a case, other than the reasons you already mentioned that
I don't agree with?

Cheers,
Conor.

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

[PATCH v2 0/1] phy: Add support for Microchip ATA6561 CAN Transceiver

[Ilya Orazov]

Ok, I changed patch to allow ti,tcan1042 as fallback compatible.

Ilya Orazov (1):
  dt-bindings: phy: ti,tcan104x-can: Document Microchip ATA6561

 .../devicetree/bindings/phy/ti,tcan104x-can.yaml    | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.34.1

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

[Ilya Orazov]

Microchip ATA6561 is High-Speed CAN Transceiver with Standby Mode.
It is pin-compatible with TI TCAN1042 and has a compatible programming
model, therefore use ATA6561 as fallback device for TCAN1042.

Signed-off-by: Ilya Orazov <ilordash02@gmail.com>
---
 .../devicetree/bindings/phy/ti,tcan104x-can.yaml    | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
index 79dad3e89aa6..f6f1fd843874 100644
--- a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
+++ b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
@@ -14,10 +14,15 @@ properties:
     pattern: "^can-phy"
 
   compatible:
-    enum:
-      - nxp,tjr1443
-      - ti,tcan1042
-      - ti,tcan1043
+    oneOf:
+      - items:
+        - enum:
+          - microchip,ata6561
+        - const: ti,tcan1042
+      - enum:
+          const: ti,tcan1042
+          const: ti,tcan1043
+          const: nxp,tjr1443
 
   '#phy-cells':
     const: 0
-- 
2.34.1

[PATCH v2 0/1] phy: Add support for Microchip ATA6561 CAN Transceiver

[Ilya Orazov]

I'm sorry, I missed mistake in commit message, fixed it.

Ilya Orazov (1):
  dt-bindings: phy: ti,tcan104x-can: Document Microchip ATA6561

 .../devicetree/bindings/phy/ti,tcan104x-can.yaml    | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.34.1

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

[Ilya Orazov]

Microchip ATA6561 is High-Speed CAN Transceiver with Standby Mode.
It is pin-compatible with TI TCAN1042 and has a compatible programming
model, therefore use ti,tcan1042 as fallback compatible.

Signed-off-by: Ilya Orazov <ilordash02@gmail.com>
---
 .../devicetree/bindings/phy/ti,tcan104x-can.yaml    | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
index 79dad3e89aa6..f6f1fd843874 100644
--- a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
+++ b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
@@ -14,10 +14,15 @@ properties:
     pattern: "^can-phy"
 
   compatible:
-    enum:
-      - nxp,tjr1443
-      - ti,tcan1042
-      - ti,tcan1043
+    oneOf:
+      - items:
+        - enum:
+          - microchip,ata6561
+        - const: ti,tcan1042
+      - enum:
+          const: ti,tcan1042
+          const: ti,tcan1043
+          const: nxp,tjr1443
 
   '#phy-cells':
     const: 0
-- 
2.34.1

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

[Rob Herring (Arm)]

On Wed, 07 Aug 2024 21:09:56 +0300, Ilya Orazov wrote:
> Microchip ATA6561 is High-Speed CAN Transceiver with Standby Mode.
> It is pin-compatible with TI TCAN1042 and has a compatible programming
> model, therefore use ti,tcan1042 as fallback compatible.
> 
> Signed-off-by: Ilya Orazov <ilordash02@gmail.com>
> ---
>  .../devicetree/bindings/phy/ti,tcan104x-can.yaml    | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml:19:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml:20:11: [warning] wrong indentation: expected 12 but found 10 (indentation)
./Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml:24:11: [error] duplication of key "const" in mapping (key-duplicates)
./Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml:25:11: [error] duplication of key "const" in mapping (key-duplicates)

dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/phy/ti,tcan104x-can.example.dts'
Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml:24:11: found duplicate key "const" with value "ti,tcan1043" (original value: "ti,tcan1042")
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/phy/ti,tcan104x-can.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml:24:11: found duplicate key "const" with value "ti,tcan1043" (original value: "ti,tcan1042")
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240807180956.1341332-2-ilordash02@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

On Wed, Aug 07, 2024 at 09:09:56PM +0300, Ilya Orazov wrote:
> Microchip ATA6561 is High-Speed CAN Transceiver with Standby Mode.
> It is pin-compatible with TI TCAN1042 and has a compatible programming
> model, therefore use ti,tcan1042 as fallback compatible.
> 
> Signed-off-by: Ilya Orazov <ilordash02@gmail.com>
> ---
>  .../devicetree/bindings/phy/ti,tcan104x-can.yaml    | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
> index 79dad3e89aa6..f6f1fd843874 100644
> --- a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
> +++ b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
> @@ -14,10 +14,15 @@ properties:
>      pattern: "^can-phy"
>  
>    compatible:
> -    enum:
> -      - nxp,tjr1443
> -      - ti,tcan1042
> -      - ti,tcan1043
> +    oneOf:
> +      - items:
> +        - enum:
> +          - microchip,ata6561
> +        - const: ti,tcan1042
> +      - enum:
> +          const: ti,tcan1042
> +          const: ti,tcan1043
> +          const: nxp,tjr1443

The enum doesn't need the "const:s", just a "-", hence the bot
complaining.

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

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

[Krzysztof Kozlowski]

On 08/08/2024 17:48, Conor Dooley wrote:
> On Wed, Aug 07, 2024 at 09:09:56PM +0300, Ilya Orazov wrote:
>> Microchip ATA6561 is High-Speed CAN Transceiver with Standby Mode.
>> It is pin-compatible with TI TCAN1042 and has a compatible programming
>> model, therefore use ti,tcan1042 as fallback compatible.
>>
>> Signed-off-by: Ilya Orazov <ilordash02@gmail.com>
>> ---
>>  .../devicetree/bindings/phy/ti,tcan104x-can.yaml    | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
>> index 79dad3e89aa6..f6f1fd843874 100644
>> --- a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
>> +++ b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
>> @@ -14,10 +14,15 @@ properties:
>>      pattern: "^can-phy"
>>  
>>    compatible:
>> -    enum:
>> -      - nxp,tjr1443
>> -      - ti,tcan1042
>> -      - ti,tcan1043
>> +    oneOf:
>> +      - items:
>> +        - enum:
>> +          - microchip,ata6561
>> +        - const: ti,tcan1042
>> +      - enum:
>> +          const: ti,tcan1042
>> +          const: ti,tcan1043
>> +          const: nxp,tjr1443
> 
> The enum doesn't need the "const:s", just a "-", hence the bot
> complaining.

Plus indentation is broken, which is important for YAML. Before posting
the patch, please test it.

Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

Best regards,
Krzysztof

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

[Ilya Orazov]

Microchip ATA6561 is High-Speed CAN Transceiver with Standby Mode.
It is pin-compatible with TI TCAN1042 and has a compatible programming
model, therefore use ti,tcan1042 as fallback compatible.

Signed-off-by: Ilya Orazov <ilordash02@gmail.com>
---
 .../devicetree/bindings/phy/ti,tcan104x-can.yaml    | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
index 79dad3e89aa6..4a8c3829d85d 100644
--- a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
+++ b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
@@ -14,10 +14,15 @@ properties:
     pattern: "^can-phy"
 
   compatible:
-    enum:
-      - nxp,tjr1443
-      - ti,tcan1042
-      - ti,tcan1043
+    oneOf:
+      - items:
+          - enum:
+              - microchip,ata6561
+          - const: ti,tcan1042
+      - enum:
+          - ti,tcan1042
+          - ti,tcan1043
+          - nxp,tjr1443
 
   '#phy-cells':
     const: 0

base-commit: 6a0e38264012809afa24113ee2162dc07f4ed22b
-- 
2.34.1

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

On Thu, Aug 08, 2024 at 10:17:35PM +0300, Ilya Orazov wrote:
> Microchip ATA6561 is High-Speed CAN Transceiver with Standby Mode.
> It is pin-compatible with TI TCAN1042 and has a compatible programming
> model, therefore use ti,tcan1042 as fallback compatible.
> 
> Signed-off-by: Ilya Orazov <ilordash02@gmail.com>
> ---

Acked-by: Conor Dooley <conor.dooley@microchip.com>

btw, please don't send new versions as a reply to existing ones, it's a
shortcut to getting lost in deep mailboxes that sort by thread (or as I
almost did here, deletion of the mail without reading it).

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

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

[Ilya Orazov]

On Thu, 8 Aug 2024 at 22:18, Ilya Orazov <ilordash02@gmail.com> wrote:
>
> Microchip ATA6561 is High-Speed CAN Transceiver with Standby Mode.
> It is pin-compatible with TI TCAN1042 and has a compatible programming
> model, therefore use ti,tcan1042 as fallback compatible.
>
> Signed-off-by: Ilya Orazov <ilordash02@gmail.com>
> ---
>  .../devicetree/bindings/phy/ti,tcan104x-can.yaml    | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
> index 79dad3e89aa6..4a8c3829d85d 100644
> --- a/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
> +++ b/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
> @@ -14,10 +14,15 @@ properties:
>      pattern: "^can-phy"
>
>    compatible:
> -    enum:
> -      - nxp,tjr1443
> -      - ti,tcan1042
> -      - ti,tcan1043
> +    oneOf:
> +      - items:
> +          - enum:
> +              - microchip,ata6561
> +          - const: ti,tcan1042
> +      - enum:
> +          - ti,tcan1042
> +          - ti,tcan1043
> +          - nxp,tjr1443
>
>    '#phy-cells':
>      const: 0
>
> base-commit: 6a0e38264012809afa24113ee2162dc07f4ed22b
> --
> 2.34.1
>

Could you please review my patch?

I hope the new patch version hasn't been lost in your inbox. Thanks to
Conor, I understand now that sending new versions as a reply wasn't
the best approach. I appreciate your time and feedback.

-- 
Best regards,
Ilya Orazov

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

[Krzysztof Kozlowski]

On 13/08/2024 19:14, Ilya Orazov wrote:
>>    '#phy-cells':
>>      const: 0
>>
>> base-commit: 6a0e38264012809afa24113ee2162dc07f4ed22b
>> --
>> 2.34.1
>>
> 
> Could you please review my patch?

You received review. Why do you ping after few days?

Best regards,
Krzysztof

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

[Ilya Orazov]

On Wed, 14 Aug 2024 at 12:03, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 13/08/2024 19:14, Ilya Orazov wrote:
> >>    '#phy-cells':
> >>      const: 0
> >>
> >> base-commit: 6a0e38264012809afa24113ee2162dc07f4ed22b
> >> --
> >> 2.34.1
> >>
> >
> > Could you please review my patch?
>
> You received review. Why do you ping after few days?

I have uploaded new patch version and fixed all dt-binding errors, so
I thought you might want to review this updated version.

-- 
Best regards,
Ilya Orazov