Add YAML DT Schema for the AMD w1 master IP.
This hardware guarantees protocol timing for driving off-board devices such
as thermal sensors, proms, etc using the 1wire protocol.
Add MAINTAINERS entry for DT Schema.
Co-developed-by: Thomas Delev <[email protected]>
Signed-off-by: Thomas Delev <[email protected]>
Signed-off-by: Kris Chaplin <[email protected]>
---
.../bindings/w1/amd,axi-1wire-master.yaml | 44 +++++++++++++++++++
MAINTAINERS | 7 +++
2 files changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
diff --git a/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml b/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
new file mode 100644
index 000000000000..41f7294a84a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/w1/amd,axi-1wire-master.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD AXI 1-wire bus master for Programmable Logic
+
+maintainers:
+ - Kris Chaplin <[email protected]>
+
+properties:
+ compatible:
+ const: amd,axi-1wire-master
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ onewire@a0000000 {
+ compatible = "amd,axi-1wire-master";
+ reg = <0xa0000000 0x10000>;
+ clocks = <&zynqmp_clk 0x47>;
+ interrupts = <GIC_SPI 0x59 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 4b2c378b4fd9..6ec3922b256e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1066,6 +1066,13 @@ M: Sanjay R Mehta <[email protected]>
S: Maintained
F: drivers/spi/spi-amd.c
+AMD W1 DRIVER
+M: Kris Chaplin <[email protected]>
+R: Thomas Delev <[email protected]>
+R: Michal Simek <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
+
AMD XGBE DRIVER
M: "Shyam Sundar S K" <[email protected]>
L: [email protected]
--
2.42.GIT
On Fri, Oct 13, 2023 at 02:30:12AM -0700, Kris Chaplin wrote:
> Add YAML DT Schema for the AMD w1 master IP.
>
> This hardware guarantees protocol timing for driving off-board devices such
> as thermal sensors, proms, etc using the 1wire protocol.
>
> Add MAINTAINERS entry for DT Schema.
>
> Co-developed-by: Thomas Delev <[email protected]>
> Signed-off-by: Thomas Delev <[email protected]>
> Signed-off-by: Kris Chaplin <[email protected]>
> ---
> .../bindings/w1/amd,axi-1wire-master.yaml | 44 +++++++++++++++++++
> MAINTAINERS | 7 +++
> 2 files changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
>
> diff --git a/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml b/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
> new file mode 100644
> index 000000000000..41f7294a84a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/w1/amd,axi-1wire-master.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AMD AXI 1-wire bus master for Programmable Logic
Inconsistent case for the title here bothers my OCD, but the binding is
fine as far as I can see.
Reviewed-by: Conor Dooley <[email protected]>
Thanks,
Conor.
> +
> +maintainers:
> + - Kris Chaplin <[email protected]>
> +
> +properties:
> + compatible:
> + const: amd,axi-1wire-master
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + onewire@a0000000 {
> + compatible = "amd,axi-1wire-master";
> + reg = <0xa0000000 0x10000>;
> + clocks = <&zynqmp_clk 0x47>;
> + interrupts = <GIC_SPI 0x59 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4b2c378b4fd9..6ec3922b256e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1066,6 +1066,13 @@ M: Sanjay R Mehta <[email protected]>
> S: Maintained
> F: drivers/spi/spi-amd.c
>
> +AMD W1 DRIVER
> +M: Kris Chaplin <[email protected]>
> +R: Thomas Delev <[email protected]>
> +R: Michal Simek <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
> +
> AMD XGBE DRIVER
> M: "Shyam Sundar S K" <[email protected]>
> L: [email protected]
> --
> 2.42.GIT
>
On 13/10/2023 11:30, Kris Chaplin wrote:
> Add YAML DT Schema for the AMD w1 master IP.
>
> This hardware guarantees protocol timing for driving off-board devices such
> as thermal sensors, proms, etc using the 1wire protocol.
>
> Add MAINTAINERS entry for DT Schema.
>
> Co-developed-by: Thomas Delev <[email protected]>
> Signed-off-by: Thomas Delev <[email protected]>
> Signed-off-by: Kris Chaplin <[email protected]>
> ---
> .../bindings/w1/amd,axi-1wire-master.yaml | 44 +++++++++++++++++++
> MAINTAINERS | 7 +++
> 2 files changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
>
> diff --git a/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml b/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
> new file mode 100644
> index 000000000000..41f7294a84a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/w1/amd,axi-1wire-master.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AMD AXI 1-wire bus master for Programmable Logic
> +
> +maintainers:
> + - Kris Chaplin <[email protected]>
> +
> +properties:
> + compatible:
> + const: amd,axi-1wire-master
That's a quite generic compatible. axi is ARM term, 1-wire is the name
of the bus and master is the role. Concatenating three common words does
not create unique device name. Compatibles are supposed to be specific
and this is really relaxed. Anything can be over AXI, everything in
1wire is 1wire and every master device is a master.
Best regards,
Krzysztof
On Fri, Oct 13, 2023 at 05:04:32PM +0200, Krzysztof Kozlowski wrote:
> On 13/10/2023 11:30, Kris Chaplin wrote:
> > Add YAML DT Schema for the AMD w1 master IP.
> >
> > This hardware guarantees protocol timing for driving off-board devices such
> > as thermal sensors, proms, etc using the 1wire protocol.
> >
> > Add MAINTAINERS entry for DT Schema.
> >
> > Co-developed-by: Thomas Delev <[email protected]>
> > Signed-off-by: Thomas Delev <[email protected]>
> > Signed-off-by: Kris Chaplin <[email protected]>
> > ---
> > .../bindings/w1/amd,axi-1wire-master.yaml | 44 +++++++++++++++++++
> > MAINTAINERS | 7 +++
> > 2 files changed, 51 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml b/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
> > new file mode 100644
> > index 000000000000..41f7294a84a3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/w1/amd,axi-1wire-master.yaml
> > @@ -0,0 +1,44 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/w1/amd,axi-1wire-master.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: AMD AXI 1-wire bus master for Programmable Logic
> > +
> > +maintainers:
> > + - Kris Chaplin <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + const: amd,axi-1wire-master
>
> That's a quite generic compatible. axi is ARM term, 1-wire is the name
> of the bus and master is the role. Concatenating three common words does
> not create unique device name. Compatibles are supposed to be specific
> and this is really relaxed. Anything can be over AXI, everything in
> 1wire is 1wire and every master device is a master.
Given the vendor (and the title of the binding) this is almost certainly
an FPGA IP core, so the generic name is understandable. Using the exact
name of the IP in the AMD/Xilinx catalog probably is the best choice?
On 13/10/2023 17:07, Conor Dooley wrote:
>>> +maintainers:
>>> + - Kris Chaplin <[email protected]>
>>> +
>>> +properties:
>>> + compatible:
>>> + const: amd,axi-1wire-master
>>
>> That's a quite generic compatible. axi is ARM term, 1-wire is the name
>> of the bus and master is the role. Concatenating three common words does
>> not create unique device name. Compatibles are supposed to be specific
>> and this is really relaxed. Anything can be over AXI, everything in
>> 1wire is 1wire and every master device is a master.
>
> Given the vendor (and the title of the binding) this is almost certainly
> an FPGA IP core, so the generic name is understandable. Using the exact
> name of the IP in the AMD/Xilinx catalog probably is the best choice?
Other option is that it is a part of some Zynq SoC.
Best regards,
Krzysztof
On 13/10/2023 16:07, Conor Dooley wrote:
> On Fri, Oct 13, 2023 at 05:04:32PM +0200, Krzysztof Kozlowski wrote:
>>
>> That's a quite generic compatible. axi is ARM term, 1-wire is the name
>> of the bus and master is the role. Concatenating three common words does
>> not create unique device name. Compatibles are supposed to be specific
>> and this is really relaxed. Anything can be over AXI, everything in
>> 1wire is 1wire and every master device is a master.
> Given the vendor (and the title of the binding) this is almost certainly
> an FPGA IP core, so the generic name is understandable. Using the exact
> name of the IP in the AMD/Xilinx catalog probably is the best choice?
Indeed this is an Programmable Logic IP core - the official name of the
core in our catalog is axi_1wire_master. It is a soft HDL core.
regards,
Kris
On 13/10/2023 17:23, Kris Chaplin wrote:
>
> On 13/10/2023 16:07, Conor Dooley wrote:
>> On Fri, Oct 13, 2023 at 05:04:32PM +0200, Krzysztof Kozlowski wrote:
>>>
>>> That's a quite generic compatible. axi is ARM term, 1-wire is the name
>>> of the bus and master is the role. Concatenating three common words does
>>> not create unique device name. Compatibles are supposed to be specific
>>> and this is really relaxed. Anything can be over AXI, everything in
>>> 1wire is 1wire and every master device is a master.
>> Given the vendor (and the title of the binding) this is almost certainly
>> an FPGA IP core, so the generic name is understandable. Using the exact
>> name of the IP in the AMD/Xilinx catalog probably is the best choice?
>
> Indeed this is an Programmable Logic IP core - the official name of the
> core in our catalog is axi_1wire_master. It is a soft HDL core.
AMD product managers are highly skilled in naming things. Sigh.
Go ahead with AXI 1-wire master. Any future - from now to next 100 years
- product from AMD which will be different but sold under the same name,
thus creating conflict in compatible naming, should be rejected because
of that conflict or renamed to something else. If that happen I will
propose a name like "banana-wire".
Best regards,
Krzysztof
On 13/10/2023 16:29, Krzysztof Kozlowski wrote:
>
> AMD product managers are highly skilled in naming things. Sigh.
>
> Go ahead with AXI 1-wire master. Any future - from now to next 100 years
> - product from AMD which will be different but sold under the same name,
> thus creating conflict in compatible naming, should be rejected because
> of that conflict or renamed to something else. If that happen I will
> propose a name like "banana-wire".
>
> Best regards,
> Krzysztof
To be fair on the product managers the IP naming from a hardware
perspective is in the context of the ex-Xilinx development tools, and as
such there is no risk of namespace overlap with the rest of the
business. I'm not against changing the binding name for clarity in the
kernel. Would adding pl within the binding (Programmable Logic) assist?
Ie: amd,pl-axi-1wire-master
Regards
Kris
On Fri, Oct 13, 2023 at 04:23:15PM +0100, Kris Chaplin wrote:
>
> On 13/10/2023 16:07, Conor Dooley wrote:
> > On Fri, Oct 13, 2023 at 05:04:32PM +0200, Krzysztof Kozlowski wrote:
> > >
> > > That's a quite generic compatible. axi is ARM term, 1-wire is the name
> > > of the bus and master is the role. Concatenating three common words does
> > > not create unique device name. Compatibles are supposed to be specific
> > > and this is really relaxed. Anything can be over AXI, everything in
> > > 1wire is 1wire and every master device is a master.
> > Given the vendor (and the title of the binding) this is almost certainly
> > an FPGA IP core, so the generic name is understandable. Using the exact
> > name of the IP in the AMD/Xilinx catalog probably is the best choice?
>
> Indeed this is an Programmable Logic IP core - the official name of the core
> in our catalog is axi_1wire_master.? It is a soft HDL core.
Only 1 version of it (ever)? Like other PL IP, it needs a version number
(and not v1, v2, etc. made up by you). Really, your versioning scheme
should be documented (like
bindings/sifive/sifive-blocks-ip-versioning.txt), but Xilinx started
versioning stuff some time back.
Also, 'master' is not considered great terminology nowadays. Perhaps the
catalog name should be updated.
Rob
On 13/10/2023 18:18, Rob Herring wrote:
> i_1wire_master. It is a soft HDL core.
> Only 1 version of it (ever)? Like other PL IP, it needs a version number
> (and not v1, v2, etc. made up by you). Really, your versioning scheme
> should be documented (like
> bindings/sifive/sifive-blocks-ip-versioning.txt), but Xilinx started
> versioning stuff some time back.
I've specified the PL IP to have both an ID and version number register
in it, which is queried by the driver on probe. As such we can version
autodiscover. Should there be incompatibilty in the driver due to new
features in future PL IP, the major version number in the register will
increment and the same driver can be extended to support the modified
behaviour. The default code in this first version will check to ensure
major is at 1. I'm working with our IP group to encourage that all new
IP have this mechanism moving forwards, as registers are a lot cheaper
in logic than they used to be.
+ if (ver_major != 1) {
+ dev_err(dev, "AMD AXI W1 Master version %u.%u is not supported by
this driver",
+ ver_major, ver_minor);
+ return -ENODEV;
+ }
> Also, 'master' is not considered great terminology nowadays. Perhaps the
> catalog name should be updated.
Agreed - I've used the term to fit with others in the subsystem. Would
this be something that is changed and aligned across W1? If so I'm
happy to get the HDL IP renamed and binding / driver documentation to
match if an alternative designator to replace master has already been
chosen and standardised across 1Wire.
Regards
Kris