This patch series converts atmel-flexcom bindings into json-schema format.
Adds support for lan966x flexcom chip-select configurations and its
DT bindings.
Kavyasree Kotagiri (3):
dt-bindings: mfd: atmel,flexcom: Convert to json-schema
dt-bindings: mfd: atmel,flexcom: Add new compatible string for lan966x
mfd: atmel-flexcom: Add support for lan966x flexcom chip-select
configuration
.../bindings/mfd/atmel,flexcom.yaml | 116 ++++++++++++++++++
.../devicetree/bindings/mfd/atmel-flexcom.txt | 63 ----------
drivers/mfd/atmel-flexcom.c | 86 ++++++++++++-
3 files changed, 200 insertions(+), 65 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
--
2.17.1
LAN966x SoC flexcoms has two optional I/O lines. Namely, CS0 and CS1 in
flexcom SPI mode. CTS and RTS in flexcom USART mode. These pins
can be mapped to lan966x FLEXCOM_SHARED[0-20] pins and usage depends on
functions being configured.
Signed-off-by: Kavyasree Kotagiri <[email protected]>
---
.../bindings/mfd/atmel,flexcom.yaml | 21 ++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
index 221bd840b49e..6050482ad8ef 100644
--- a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
+++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
@@ -16,7 +16,9 @@ description:
properties:
compatible:
- const: atmel,sama5d2-flexcom
+ enum:
+ - atmel,sama5d2-flexcom
+ - microchip,lan966x-flexcom
reg:
maxItems: 1
@@ -46,6 +48,21 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32
enum: [1, 2, 3]
+ microchip,flx-shrd-pins:
+ description: Specify the Flexcom shared pins to be used for flexcom
+ chip-selects.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 20
+
+ microchip,flx-cs-names:
+ description: Chip select names. "cts", "rts" for flexcom USART "CTS" and
+ "RTS" lines. "cs0", "cs1" for flexcom SPI chip-select lines.
+ items:
+ enum: [ cs0, cs1, cts, rts ]
+ minItems: 1
+ maxItems: 2
+
patternProperties:
"^serial@[0-9a-f]+$":
description: See atmel-usart.txt for details of USART bindings.
@@ -80,6 +97,8 @@ examples:
#size-cells = <1>;
ranges = <0x0 0xf8034000 0x800>;
atmel,flexcom-mode = <2>;
+ microchip,flx-shrd-pins = <9>;
+ microchip,flx-cs-names = "cs0";
spi0: spi@400 {
compatible = "atmel,at91rm9200-spi";
--
2.17.1
Convert the Atmel flexcom device tree bindings to json schema.
Signed-off-by: Kavyasree Kotagiri <[email protected]>
---
.../bindings/mfd/atmel,flexcom.yaml | 97 +++++++++++++++++++
.../devicetree/bindings/mfd/atmel-flexcom.txt | 63 ------------
2 files changed, 97 insertions(+), 63 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
new file mode 100644
index 000000000000..221bd840b49e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/atmel,flexcom.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Device tree bindings for Atmel Flexcom (Flexible Serial Communication Unit)
+
+maintainers:
+ - Kavyasree Kotagiri <[email protected]>
+
+description:
+ The Atmel Flexcom is just a wrapper which embeds a SPI controller,
+ an I2C controller and an USART. Only one function can be used at a
+ time and is chosen at boot time according to the device tree.
+
+properties:
+ compatible:
+ const: atmel,sama5d2-flexcom
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 1
+
+ ranges:
+ description:
+ One range for the full I/O register region. (including USART,
+ TWI and SPI registers).
+ items:
+ maxItems: 3
+
+ atmel,flexcom-mode:
+ description: |
+ Specifies the flexcom mode as follows:
+ 1: USART
+ 2: SPI
+ 3: I2C.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [1, 2, 3]
+
+patternProperties:
+ "^serial@[0-9a-f]+$":
+ description: See atmel-usart.txt for details of USART bindings.
+ type: object
+
+ "^spi@[0-9a-f]+$":
+ description: See ../spi/spi_atmel.txt for details of SPI bindings.
+ type: object
+
+ "^i2c@[0-9a-f]+$":
+ description: See ../i2c/i2c-at91.txt for details of I2C bindings.
+ type: object
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - "#address-cells"
+ - "#size-cells"
+ - ranges
+ - atmel,flexcom-mode
+
+additionalProperties: false
+
+examples:
+ - |
+ flx0: flexcom@f8034000 {
+ compatible = "atmel,sama5d2-flexcom";
+ reg = <0xf8034000 0x200>;
+ clocks = <&flx0_clk>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0xf8034000 0x800>;
+ atmel,flexcom-mode = <2>;
+
+ spi0: spi@400 {
+ compatible = "atmel,at91rm9200-spi";
+ reg = <0x400 0x200>;
+ interrupts = <19 4 7>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flx0_default>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clocks = <&flx0_clk>;
+ clock-names = "spi_clk";
+ atmel,fifo-size = <32>;
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
deleted file mode 100644
index 9d837535637b..000000000000
--- a/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
+++ /dev/null
@@ -1,63 +0,0 @@
-* Device tree bindings for Atmel Flexcom (Flexible Serial Communication Unit)
-
-The Atmel Flexcom is just a wrapper which embeds a SPI controller, an I2C
-controller and an USART. Only one function can be used at a time and is chosen
-at boot time according to the device tree.
-
-Required properties:
-- compatible: Should be "atmel,sama5d2-flexcom"
-- reg: Should be the offset/length value for Flexcom dedicated
- I/O registers (without USART, TWI or SPI registers).
-- clocks: Should be the Flexcom peripheral clock from PMC.
-- #address-cells: Should be <1>
-- #size-cells: Should be <1>
-- ranges: Should be one range for the full I/O register region
- (including USART, TWI and SPI registers).
-- atmel,flexcom-mode: Should be one of the following values:
- - <1> for USART
- - <2> for SPI
- - <3> for I2C
-
-Required child:
-A single available child device of type matching the "atmel,flexcom-mode"
-property.
-
-The phandle provided by the clocks property of the child is the same as one for
-the Flexcom parent.
-
-For other properties, please refer to the documentations of the respective
-device:
-- ../serial/atmel-usart.txt
-- ../spi/spi_atmel.txt
-- ../i2c/i2c-at91.txt
-
-Example:
-
-flexcom@f8034000 {
- compatible = "atmel,sama5d2-flexcom";
- reg = <0xf8034000 0x200>;
- clocks = <&flx0_clk>;
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0x0 0xf8034000 0x800>;
- atmel,flexcom-mode = <2>;
-
- spi@400 {
- compatible = "atmel,at91rm9200-spi";
- reg = <0x400 0x200>;
- interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_flx0_default>;
- #address-cells = <1>;
- #size-cells = <0>;
- clocks = <&flx0_clk>;
- clock-names = "spi_clk";
- atmel,fifo-size = <32>;
-
- flash@0 {
- compatible = "atmel,at25f512b";
- reg = <0>;
- spi-max-frequency = <20000000>;
- };
- };
-};
--
2.17.1
On Fri, Jun 03, 2022 at 05:48:01PM +0530, Kavyasree Kotagiri wrote:
> LAN966x SoC flexcoms has two optional I/O lines. Namely, CS0 and CS1 in
> flexcom SPI mode. CTS and RTS in flexcom USART mode. These pins
> can be mapped to lan966x FLEXCOM_SHARED[0-20] pins and usage depends on
> functions being configured.
>
> Signed-off-by: Kavyasree Kotagiri <[email protected]>
> ---
> .../bindings/mfd/atmel,flexcom.yaml | 21 ++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> index 221bd840b49e..6050482ad8ef 100644
> --- a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> @@ -16,7 +16,9 @@ description:
>
> properties:
> compatible:
> - const: atmel,sama5d2-flexcom
> + enum:
> + - atmel,sama5d2-flexcom
> + - microchip,lan966x-flexcom
>
> reg:
> maxItems: 1
> @@ -46,6 +48,21 @@ properties:
> $ref: /schemas/types.yaml#/definitions/uint32
> enum: [1, 2, 3]
>
> + microchip,flx-shrd-pins:
> + description: Specify the Flexcom shared pins to be used for flexcom
> + chip-selects.
> + $ref: /schemas/types.yaml#/definitions/uint32
The driver seems to think this is an array.
> + minimum: 0
> + maximum: 20
> +
> + microchip,flx-cs-names:
> + description: Chip select names. "cts", "rts" for flexcom USART "CTS" and
> + "RTS" lines. "cs0", "cs1" for flexcom SPI chip-select lines.
> + items:
> + enum: [ cs0, cs1, cts, rts ]
> + minItems: 1
> + maxItems: 2
> +
> patternProperties:
> "^serial@[0-9a-f]+$":
> description: See atmel-usart.txt for details of USART bindings.
> @@ -80,6 +97,8 @@ examples:
> #size-cells = <1>;
> ranges = <0x0 0xf8034000 0x800>;
> atmel,flexcom-mode = <2>;
> + microchip,flx-shrd-pins = <9>;
> + microchip,flx-cs-names = "cs0";
>
> spi0: spi@400 {
> compatible = "atmel,at91rm9200-spi";
> --
> 2.17.1
>
>
LAN966x SoC have 5 flexcoms. Each flexcom has 2 chip-selects.
For each chip select of each flexcom there is a configuration
register FLEXCOM_SHARED[0-4]:SS_MASK[0-1]. The width of
configuration register is 21 because there are 21 shared pins
on each of which the chip select can be mapped. Each bit of the
register represents a different FLEXCOM_SHARED pin.
Signed-off-by: Kavyasree Kotagiri <[email protected]>
---
drivers/mfd/atmel-flexcom.c | 86 ++++++++++++++++++++++++++++++++++++-
1 file changed, 84 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
index 33caa4fba6af..f87ee3606eb0 100644
--- a/drivers/mfd/atmel-flexcom.c
+++ b/drivers/mfd/atmel-flexcom.c
@@ -28,15 +28,64 @@
#define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) & \
FLEX_MR_OPMODE_MASK)
+/* LAN966x flexcom shared register offsets */
+#define FLEX_SHRD_SS_MASK_0 0x0
+#define FLEX_SHRD_SS_MASK_1 0x4
+#define FLEX_SHRD_MASK 0x1FFFFF
+
+struct atmel_flex_caps {
+ bool has_flx_cs;
+};
+
struct atmel_flexcom {
- void __iomem *base;
+ void __iomem *base, *flexcom_shared_base;
u32 opmode;
struct clk *clk;
};
+static int atmel_flexcom_lan966x_cs_config(struct platform_device *pdev)
+{
+ struct atmel_flexcom *ddata = dev_get_drvdata(&pdev->dev);
+ struct device_node *np = pdev->dev.of_node;
+ u32 flx_shrd_pins[2], val;
+ int err, i, count;
+
+ count = of_property_count_u32_elems(np, "microchip,flx-shrd-pins");
+ if (count <= 0 || count > 2) {
+ dev_err(&pdev->dev, "Invalid %s property (%d)\n", "flx-shrd-pins",
+ count);
+ return -EINVAL;
+ }
+
+ err = of_property_read_u32_array(np, "microchip,flx-shrd-pins", flx_shrd_pins, count);
+ if (err)
+ return err;
+
+ for (i = 0; i < count; i++) {
+ const char *flx_cs;
+
+ if (flx_shrd_pins[i] > 20)
+ return -EINVAL;
+
+ val = ~(1 << flx_shrd_pins[i]) & FLEX_SHRD_MASK;
+
+ err = of_property_read_string_index(np, "microchip,flx-cs", i, &flx_cs);
+ if (err)
+ return err;
+
+ if (!strcmp(flx_cs, "cs0") || !strcmp(flx_cs, "cts"))
+ writel(val, ddata->flexcom_shared_base + FLEX_SHRD_SS_MASK_0);
+ else if (!strcmp(flx_cs, "cs1") || !strcmp(flx_cs, "rts"))
+ writel(val, ddata->flexcom_shared_base + FLEX_SHRD_SS_MASK_1);
+ }
+
+ return 0;
+}
+
static int atmel_flexcom_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
+ const struct atmel_flex_caps *caps;
struct resource *res;
struct atmel_flexcom *ddata;
int err;
@@ -76,13 +125,46 @@ static int atmel_flexcom_probe(struct platform_device *pdev)
*/
writel(FLEX_MR_OPMODE(ddata->opmode), ddata->base + FLEX_MR);
+ caps = of_device_get_match_data(&pdev->dev);
+ if (!caps) {
+ dev_err(&pdev->dev, "Could not retrieve flexcom caps\n");
+ return -EINVAL;
+ }
+
+ if (caps->has_flx_cs) {
+ ddata->flexcom_shared_base = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
+ if (IS_ERR(ddata->flexcom_shared_base))
+ return dev_err_probe(&pdev->dev,
+ PTR_ERR(ddata->flexcom_shared_base),
+ "failed to get flexcom shared base address\n");
+
+ err = atmel_flexcom_lan966x_cs_config(pdev);
+ if (err)
+ return err;
+ }
+
clk_disable_unprepare(ddata->clk);
return devm_of_platform_populate(&pdev->dev);
}
+static const struct atmel_flex_caps atmel_flexcom_caps = {};
+
+static const struct atmel_flex_caps lan966x_flexcom_caps = {
+ .has_flx_cs = true,
+};
+
static const struct of_device_id atmel_flexcom_of_match[] = {
- { .compatible = "atmel,sama5d2-flexcom" },
+ {
+ .compatible = "atmel,sama5d2-flexcom",
+ .data = &atmel_flexcom_caps,
+ },
+
+ {
+ .compatible = "microchip,lan966x-flexcom",
+ .data = &lan966x_flexcom_caps,
+ },
+
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
--
2.17.1
On 03.06.2022 15:18, Kavyasree Kotagiri wrote:
> LAN966x SoC have 5 flexcoms. Each flexcom has 2 chip-selects.
> For each chip select of each flexcom there is a configuration
> register FLEXCOM_SHARED[0-4]:SS_MASK[0-1]. The width of
> configuration register is 21 because there are 21 shared pins
> on each of which the chip select can be mapped. Each bit of the
> register represents a different FLEXCOM_SHARED pin.
>
> Signed-off-by: Kavyasree Kotagiri <[email protected]>
> ---
> drivers/mfd/atmel-flexcom.c | 86 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 84 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
> index 33caa4fba6af..f87ee3606eb0 100644
> --- a/drivers/mfd/atmel-flexcom.c
> +++ b/drivers/mfd/atmel-flexcom.c
> @@ -28,15 +28,64 @@
> #define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) & \
> FLEX_MR_OPMODE_MASK)
>
> +/* LAN966x flexcom shared register offsets */
> +#define FLEX_SHRD_SS_MASK_0 0x0
> +#define FLEX_SHRD_SS_MASK_1 0x4
> +#define FLEX_SHRD_MASK 0x1FFFFF
GENMASK() ?
> +
> +struct atmel_flex_caps {
> + bool has_flx_cs;
> +};
> +
> struct atmel_flexcom {
> - void __iomem *base;
> + void __iomem *base, *flexcom_shared_base;
Add a new line with:
void __iomem *flexcom_shared_base;
> u32 opmode;
> struct clk *clk;
> };
>
> +static int atmel_flexcom_lan966x_cs_config(struct platform_device *pdev)
> +{
> + struct atmel_flexcom *ddata = dev_get_drvdata(&pdev->dev);
> + struct device_node *np = pdev->dev.of_node;
> + u32 flx_shrd_pins[2], val;
> + int err, i, count;
> +
> + count = of_property_count_u32_elems(np, "microchip,flx-shrd-pins");
> + if (count <= 0 || count > 2) {
> + dev_err(&pdev->dev, "Invalid %s property (%d)\n", "flx-shrd-pins",
> + count);
> + return -EINVAL;
> + }
> +
> + err = of_property_read_u32_array(np, "microchip,flx-shrd-pins", flx_shrd_pins, count);
> + if (err)
> + return err;
> +
> + for (i = 0; i < count; i++) {
> + const char *flx_cs;
> +
> + if (flx_shrd_pins[i] > 20)
Could you use a macro for 20?
> + return -EINVAL;
> +
> + val = ~(1 << flx_shrd_pins[i]) & FLEX_SHRD_MASK;
> +
> + err = of_property_read_string_index(np, "microchip,flx-cs", i, &flx_cs);
Wouldn't it be better to have plain u32 constants instead of strings here?
> + if (err)
> + return err;
> +
> + if (!strcmp(flx_cs, "cs0") || !strcmp(flx_cs, "cts"))
> + writel(val, ddata->flexcom_shared_base + FLEX_SHRD_SS_MASK_0);
> + else if (!strcmp(flx_cs, "cs1") || !strcmp(flx_cs, "rts"))
> + writel(val, ddata->flexcom_shared_base + FLEX_SHRD_SS_MASK_1);
I may miss something but I don't see here the approach you introduced in [1]:
+ err = mux_control_select(flx_mux, args.args[0]);
+ if (!err) {
+ mux_control_deselect(flx_mux);
> + }
> +
> + return 0;
> +}
> +
> static int atmel_flexcom_probe(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> + const struct atmel_flex_caps *caps;
> struct resource *res;
> struct atmel_flexcom *ddata;
> int err;
> @@ -76,13 +125,46 @@ static int atmel_flexcom_probe(struct platform_device *pdev)
> */
> writel(FLEX_MR_OPMODE(ddata->opmode), ddata->base + FLEX_MR);
>
> + caps = of_device_get_match_data(&pdev->dev);
> + if (!caps) {
> + dev_err(&pdev->dev, "Could not retrieve flexcom caps\n");
> + return -EINVAL;
As I already said on [1]: you return here but the clock remain enabled.
Please take care to undo previous operations.
> + }
> +
> + if (caps->has_flx_cs) {
> + ddata->flexcom_shared_base = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
> + if (IS_ERR(ddata->flexcom_shared_base))
> + return dev_err_probe(&pdev->dev,
> + PTR_ERR(ddata->flexcom_shared_base),
> + "failed to get flexcom shared base address\n");
Ditto
> +
> + err = atmel_flexcom_lan966x_cs_config(pdev);
> + if (err)
> + return err;
Ditto
[1]
https://lore.kernel.org/lkml/[email protected]/
> + }
> +
> clk_disable_unprepare(ddata->clk);
>
> return devm_of_platform_populate(&pdev->dev);
> }
>
> +static const struct atmel_flex_caps atmel_flexcom_caps = {};
> +
> +static const struct atmel_flex_caps lan966x_flexcom_caps = {
> + .has_flx_cs = true,
> +};
> +
> static const struct of_device_id atmel_flexcom_of_match[] = {
> - { .compatible = "atmel,sama5d2-flexcom" },
> + {
> + .compatible = "atmel,sama5d2-flexcom",
> + .data = &atmel_flexcom_caps,
> + },
> +
> + {
> + .compatible = "microchip,lan966x-flexcom",
> + .data = &lan966x_flexcom_caps,
> + },
> +
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
On 03/06/2022 14:18, Kavyasree Kotagiri wrote:
> Convert the Atmel flexcom device tree bindings to json schema.
>
> Signed-off-by: Kavyasree Kotagiri <[email protected]>
> ---
> .../bindings/mfd/atmel,flexcom.yaml | 97 +++++++++++++++++++
> .../devicetree/bindings/mfd/atmel-flexcom.txt | 63 ------------
> 2 files changed, 97 insertions(+), 63 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> new file mode 100644
> index 000000000000..221bd840b49e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/atmel,flexcom.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Device tree bindings for Atmel Flexcom (Flexible Serial Communication Unit)
There was a v2 of the same patch to which already commented. Now you
send the same patch, without comments applied as v1. This does not make
any sense.
Please version your patches correctly and do not ignore received feedback.
Best regards,
Krzysztof
On 03/06/2022 14:18, Kavyasree Kotagiri wrote:
> LAN966x SoC flexcoms has two optional I/O lines. Namely, CS0 and CS1 in
> flexcom SPI mode. CTS and RTS in flexcom USART mode. These pins
> can be mapped to lan966x FLEXCOM_SHARED[0-20] pins and usage depends on
> functions being configured.
>
> Signed-off-by: Kavyasree Kotagiri <[email protected]>
> ---
> .../bindings/mfd/atmel,flexcom.yaml | 21 ++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> index 221bd840b49e..6050482ad8ef 100644
> --- a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> @@ -16,7 +16,9 @@ description:
>
> properties:
> compatible:
> - const: atmel,sama5d2-flexcom
> + enum:
> + - atmel,sama5d2-flexcom
> + - microchip,lan966x-flexcom
Your new v1 is here worse than old v2, where this was just simple
extension of existing enum. Why did you change it?
Best regards,
Krzysztof
> > Convert the Atmel flexcom device tree bindings to json schema.
> >
> > Signed-off-by: Kavyasree Kotagiri <[email protected]>
> > ---
> > .../bindings/mfd/atmel,flexcom.yaml | 97 +++++++++++++++++++
> > .../devicetree/bindings/mfd/atmel-flexcom.txt | 63 ------------
> > 2 files changed, 97 insertions(+), 63 deletions(-)
> > create mode 100644
> Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-
> flexcom.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > new file mode 100644
> > index 000000000000..221bd840b49e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > @@ -0,0 +1,97 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/atmel,flexcom.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Device tree bindings for Atmel Flexcom (Flexible Serial
> Communication Unit)
>
> There was a v2 of the same patch to which already commented. Now you
> send the same patch, without comments applied as v1. This does not make
> any sense.
>
> Please version your patches correctly and do not ignore received feedback.
>
Hi,
FYI, lan966x flexcom chip-select driver support is moved to atmel-flexcom.c rather than
implementing a new mux driver. So, I sent new version now with driver changes in drives/mfd/atmel-flexcom.c
I once again gone through your v2 comments. I missed this comment "s/Device tree bindings//".
Except that I addressed remaining all other comments for dt-bindings patches.
>
> Best regards,
> Krzysztof
> > LAN966x SoC flexcoms has two optional I/O lines. Namely, CS0 and CS1 in
> > flexcom SPI mode. CTS and RTS in flexcom USART mode. These pins
> > can be mapped to lan966x FLEXCOM_SHARED[0-20] pins and usage
> depends on
> > functions being configured.
> >
> > Signed-off-by: Kavyasree Kotagiri <[email protected]>
> > ---
> > .../bindings/mfd/atmel,flexcom.yaml | 21 ++++++++++++++++++-
> > 1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > index 221bd840b49e..6050482ad8ef 100644
> > --- a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > @@ -16,7 +16,9 @@ description:
> >
> > properties:
> > compatible:
> > - const: atmel,sama5d2-flexcom
> > + enum:
> > + - atmel,sama5d2-flexcom
> > + - microchip,lan966x-flexcom
>
> Your new v1 is here worse than old v2, where this was just simple
> extension of existing enum. Why did you change it?
>
I introduced new compatible string for lan966x and also I have new DT properties
"microchip,flx-shrd-pins" and "microchip,flx-cs-names".
>
> Best regards,
> Krzysztof
On 06/06/2022 15:28, [email protected] wrote:
>>> LAN966x SoC flexcoms has two optional I/O lines. Namely, CS0 and CS1 in
>>> flexcom SPI mode. CTS and RTS in flexcom USART mode. These pins
>>> can be mapped to lan966x FLEXCOM_SHARED[0-20] pins and usage
>> depends on
>>> functions being configured.
>>>
>>> Signed-off-by: Kavyasree Kotagiri <[email protected]>
>>> ---
>>> .../bindings/mfd/atmel,flexcom.yaml | 21 ++++++++++++++++++-
>>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>> b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>> index 221bd840b49e..6050482ad8ef 100644
>>> --- a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>> +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>> @@ -16,7 +16,9 @@ description:
>>>
>>> properties:
>>> compatible:
>>> - const: atmel,sama5d2-flexcom
>>> + enum:
>>> + - atmel,sama5d2-flexcom
>>> + - microchip,lan966x-flexcom
>>
>> Your new v1 is here worse than old v2, where this was just simple
>> extension of existing enum. Why did you change it?
>>
> I introduced new compatible string for lan966x and also I have new DT properties
> "microchip,flx-shrd-pins" and "microchip,flx-cs-names".
v1 also had the new compatible, hadn't it? The difference is in the enum
- before you did not modify this line. Less code in the diff...
Best regards,
Krzysztof
> > > Convert the Atmel flexcom device tree bindings to json schema.
> > >
> > > Signed-off-by: Kavyasree Kotagiri <[email protected]>
> > > ---
> > > .../bindings/mfd/atmel,flexcom.yaml | 97 +++++++++++++++++++
> > > .../devicetree/bindings/mfd/atmel-flexcom.txt | 63 ------------
> > > 2 files changed, 97 insertions(+), 63 deletions(-)
> > > create mode 100644
> > Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-
> > flexcom.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > > new file mode 100644
> > > index 000000000000..221bd840b49e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> > > @@ -0,0 +1,97 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mfd/atmel,flexcom.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Device tree bindings for Atmel Flexcom (Flexible Serial
> > Communication Unit)
> >
> > There was a v2 of the same patch to which already commented. Now you
> > send the same patch, without comments applied as v1. This does not make
> > any sense.
> >
> > Please version your patches correctly and do not ignore received feedback.
> >
>
> Hi,
>
> FYI, lan966x flexcom chip-select driver support is moved to atmel-flexcom.c
> rather than
> implementing a new mux driver. So, I sent new version now with driver
> changes in drives/mfd/atmel-flexcom.c
> I once again gone through your v2 comments. I missed this comment
> "s/Device tree bindings//".
> Except that I addressed remaining all other comments for dt-bindings
> patches.
> >
Hi,
Please let me know what you mean by " s/Device tree bindings//" comment?
Thanks,
Kavya
> > Best regards,
> > Krzysztof
On 07/06/2022 08:55, [email protected] wrote:
>>>> Convert the Atmel flexcom device tree bindings to json schema.
>>>>
>>>> Signed-off-by: Kavyasree Kotagiri <[email protected]>
>>>> ---
>>>> .../bindings/mfd/atmel,flexcom.yaml | 97 +++++++++++++++++++
>>>> .../devicetree/bindings/mfd/atmel-flexcom.txt | 63 ------------
>>>> 2 files changed, 97 insertions(+), 63 deletions(-)
>>>> create mode 100644
>>> Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>>> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-
>>> flexcom.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>> b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>>> new file mode 100644
>>>> index 000000000000..221bd840b49e
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>>> @@ -0,0 +1,97 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/mfd/atmel,flexcom.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Device tree bindings for Atmel Flexcom (Flexible Serial
>>> Communication Unit)
>>>
>>> There was a v2 of the same patch to which already commented. Now you
>>> send the same patch, without comments applied as v1. This does not make
>>> any sense.
>>>
>>> Please version your patches correctly and do not ignore received feedback.
>>>
>>
>> Hi,
>>
>> FYI, lan966x flexcom chip-select driver support is moved to atmel-flexcom.c
>> rather than
>> implementing a new mux driver. So, I sent new version now with driver
>> changes in drives/mfd/atmel-flexcom.c
>> I once again gone through your v2 comments. I missed this comment
>> "s/Device tree bindings//".
>> Except that I addressed remaining all other comments for dt-bindings
>> patches.
>>>
> Hi,
>
> Please let me know what you mean by " s/Device tree bindings//" comment?
s/X/Y/ is search and replace X with Y, so here, please remove the
"Device tree bindings" words.
Best regards,
Krzysztof
On 07/06/2022 09:54, [email protected] wrote:
>>>>> LAN966x SoC flexcoms has two optional I/O lines. Namely, CS0 and CS1
>> in
>>>>> flexcom SPI mode. CTS and RTS in flexcom USART mode. These pins
>>>>> can be mapped to lan966x FLEXCOM_SHARED[0-20] pins and usage
>>>> depends on
>>>>> functions being configured.
>>>>>
>>>>> Signed-off-by: Kavyasree Kotagiri <[email protected]>
>>>>> ---
>>>>> .../bindings/mfd/atmel,flexcom.yaml | 21 ++++++++++++++++++-
>>>>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>>> b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>>>> index 221bd840b49e..6050482ad8ef 100644
>>>>> --- a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>>>> +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
>>>>> @@ -16,7 +16,9 @@ description:
>>>>>
>>>>> properties:
>>>>> compatible:
>>>>> - const: atmel,sama5d2-flexcom
>>>>> + enum:
>>>>> + - atmel,sama5d2-flexcom
>>>>> + - microchip,lan966x-flexcom
>>>>
>>>> Your new v1 is here worse than old v2, where this was just simple
>>>> extension of existing enum. Why did you change it?
>>>>
>>> I introduced new compatible string for lan966x and also I have new DT
>> properties
>>> "microchip,flx-shrd-pins" and "microchip,flx-cs-names".
>>
>> v1 also had the new compatible, hadn't it? The difference is in the enum
>> - before you did not modify this line. Less code in the diff...
>>
> Yes, previous patch series also had new compatible which introduced new mux DT properties in atmel-flexcom.yaml and mux driver.
> As part of review comments from Peter Rosin on driver part, Mux implementation is dropped. Please ignore previous patch series. Now, flexcom chip-select support is done in atme-flexcom.c driver. So, I started new patch series now which introduced new DT properties "microchip,flx-shrd-pins" and "microchip,flx-cs-names" and driver changes only in atmel-flexcom.c driver.
We talk only about the hunk here, not about other properties.
Best regards,
Krzysztof
> >>> LAN966x SoC flexcoms has two optional I/O lines. Namely, CS0 and CS1
> in
> >>> flexcom SPI mode. CTS and RTS in flexcom USART mode. These pins
> >>> can be mapped to lan966x FLEXCOM_SHARED[0-20] pins and usage
> >> depends on
> >>> functions being configured.
> >>>
> >>> Signed-off-by: Kavyasree Kotagiri <[email protected]>
> >>> ---
> >>> .../bindings/mfd/atmel,flexcom.yaml | 21 ++++++++++++++++++-
> >>> 1 file changed, 20 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> >> b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> >>> index 221bd840b49e..6050482ad8ef 100644
> >>> --- a/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> >>> +++ b/Documentation/devicetree/bindings/mfd/atmel,flexcom.yaml
> >>> @@ -16,7 +16,9 @@ description:
> >>>
> >>> properties:
> >>> compatible:
> >>> - const: atmel,sama5d2-flexcom
> >>> + enum:
> >>> + - atmel,sama5d2-flexcom
> >>> + - microchip,lan966x-flexcom
> >>
> >> Your new v1 is here worse than old v2, where this was just simple
> >> extension of existing enum. Why did you change it?
> >>
> > I introduced new compatible string for lan966x and also I have new DT
> properties
> > "microchip,flx-shrd-pins" and "microchip,flx-cs-names".
>
> v1 also had the new compatible, hadn't it? The difference is in the enum
> - before you did not modify this line. Less code in the diff...
>
Yes, previous patch series also had new compatible which introduced new mux DT properties in atmel-flexcom.yaml and mux driver.
As part of review comments from Peter Rosin on driver part, Mux implementation is dropped. Please ignore previous patch series. Now, flexcom chip-select support is done in atme-flexcom.c driver. So, I started new patch series now which introduced new DT properties "microchip,flx-shrd-pins" and "microchip,flx-cs-names" and driver changes only in atmel-flexcom.c driver.
>
> Best regards,
> Krzysztof