devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios
@ 2024-06-03 12:37 Hui Wang
  2024-06-03 12:37 ` [PATCH 2/2] serial: sc16is7xx: setup reset pin if it is defined in device tree Hui Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Hui Wang @ 2024-06-03 12:37 UTC (permalink / raw)
  To: linux-serial, devicetree, gregkh; +Cc: jirislaby, hvilleneuve, hui.wang

In some designs, the chip reset pin is connected to a gpio, this
gpio needs to be set correctly before probing the driver, so adding
a reset-gpios in the device tree.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
index 5dec15b7e7c3..62aff6e034cb 100644
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
@@ -28,6 +28,9 @@ properties:
   clocks:
     maxItems: 1
 
+  reset-gpios:
+    maxItems: 1
+
   clock-frequency:
     description:
       When there is no clock provider visible to the platform, this
@@ -120,6 +123,7 @@ examples:
             compatible = "nxp,sc16is752";
             reg = <0x54>;
             clocks = <&clk20m>;
+            reset-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
             interrupt-parent = <&gpio3>;
             interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
             nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] serial: sc16is7xx: setup reset pin if it is defined in device tree
  2024-06-03 12:37 [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios Hui Wang
@ 2024-06-03 12:37 ` Hui Wang
  2024-06-03 13:11   ` Hugo Villeneuve
  2024-06-03 13:18 ` [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios Rob Herring
  2024-06-04  6:54 ` Krzysztof Kozlowski
  2 siblings, 1 reply; 8+ messages in thread
From: Hui Wang @ 2024-06-03 12:37 UTC (permalink / raw)
  To: linux-serial, devicetree, gregkh; +Cc: jirislaby, hvilleneuve, hui.wang

Certain designs connect a gpio to the reset pin, and the reset pin
needs to be setup correctly before accessing the chip.

Here adding a function to handle the reset pin. This change has no
impact if there is no reset_gpios defined in the device tree.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/tty/serial/sc16is7xx.c     | 22 ++++++++++++++++++++++
 drivers/tty/serial/sc16is7xx.h     |  2 ++
 drivers/tty/serial/sc16is7xx_i2c.c |  2 ++
 drivers/tty/serial/sc16is7xx_spi.c |  2 ++
 4 files changed, 28 insertions(+)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index bf0065d1c8e9..53bfb603b03c 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -19,6 +19,7 @@
 #include <linux/kthread.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/of_gpio.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/sched.h>
@@ -1467,6 +1468,27 @@ static const struct serial_rs485 sc16is7xx_rs485_supported = {
 	.delay_rts_after_send = 1,	/* Not supported but keep returning -EINVAL */
 };
 
+void sc16is7xx_setup_reset_pin(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	int reset_gpio, err;
+
+	reset_gpio = of_get_named_gpio(np, "reset-gpios", 0);
+	if (!gpio_is_valid(reset_gpio))
+		return;
+
+	err = devm_gpio_request_one(dev, reset_gpio, GPIOF_OUT_INIT_LOW,
+				    "sc16is7xx-reset");
+	if (err) {
+		dev_err(dev, "failed to request sc16is7xx-reset-gpios: %d\n", err);
+		return;
+	}
+
+	/* Deassert the reset pin */
+	gpio_set_value_cansleep(reset_gpio, 1);
+}
+EXPORT_SYMBOL_GPL(sc16is7xx_setup_reset_pin);
+
 int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 		    struct regmap *regmaps[], int irq)
 {
diff --git a/drivers/tty/serial/sc16is7xx.h b/drivers/tty/serial/sc16is7xx.h
index afb784eaee45..f4ae114cc41a 100644
--- a/drivers/tty/serial/sc16is7xx.h
+++ b/drivers/tty/serial/sc16is7xx.h
@@ -33,6 +33,8 @@ const char *sc16is7xx_regmap_name(u8 port_id);
 
 unsigned int sc16is7xx_regmap_port_mask(unsigned int port_id);
 
+void sc16is7xx_setup_reset_pin(struct device *dev);
+
 int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 		    struct regmap *regmaps[], int irq);
 
diff --git a/drivers/tty/serial/sc16is7xx_i2c.c b/drivers/tty/serial/sc16is7xx_i2c.c
index 3ed47c306d85..9833c3b935c2 100644
--- a/drivers/tty/serial/sc16is7xx_i2c.c
+++ b/drivers/tty/serial/sc16is7xx_i2c.c
@@ -21,6 +21,8 @@ static int sc16is7xx_i2c_probe(struct i2c_client *i2c)
 	if (!devtype)
 		return dev_err_probe(&i2c->dev, -ENODEV, "Failed to match device\n");
 
+	sc16is7xx_setup_reset_pin(&i2c->dev);
+
 	memcpy(&regcfg, &sc16is7xx_regcfg, sizeof(struct regmap_config));
 
 	for (i = 0; i < devtype->nr_uart; i++) {
diff --git a/drivers/tty/serial/sc16is7xx_spi.c b/drivers/tty/serial/sc16is7xx_spi.c
index 73df36f8a7fd..ce38561faaf0 100644
--- a/drivers/tty/serial/sc16is7xx_spi.c
+++ b/drivers/tty/serial/sc16is7xx_spi.c
@@ -38,6 +38,8 @@ static int sc16is7xx_spi_probe(struct spi_device *spi)
 	if (!devtype)
 		return dev_err_probe(&spi->dev, -ENODEV, "Failed to match device\n");
 
+	sc16is7xx_setup_reset_pin(&spi->dev);
+
 	memcpy(&regcfg, &sc16is7xx_regcfg, sizeof(struct regmap_config));
 
 	for (i = 0; i < devtype->nr_uart; i++) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] serial: sc16is7xx: setup reset pin if it is defined in device tree
  2024-06-03 12:37 ` [PATCH 2/2] serial: sc16is7xx: setup reset pin if it is defined in device tree Hui Wang
@ 2024-06-03 13:11   ` Hugo Villeneuve
  2024-06-04 10:34     ` Hui Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Hugo Villeneuve @ 2024-06-03 13:11 UTC (permalink / raw)
  To: Hui Wang; +Cc: linux-serial, devicetree, gregkh, jirislaby, hvilleneuve

On Mon,  3 Jun 2024 20:37:10 +0800
Hui Wang <hui.wang@canonical.com> wrote:

Hi Hui,

> Certain designs connect a gpio to the reset pin, and the reset pin
> needs to be setup correctly before accessing the chip.
> 
> Here adding a function to handle the reset pin. This change has no
> impact if there is no reset_gpios defined in the device tree.
> 
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  drivers/tty/serial/sc16is7xx.c     | 22 ++++++++++++++++++++++
>  drivers/tty/serial/sc16is7xx.h     |  2 ++
>  drivers/tty/serial/sc16is7xx_i2c.c |  2 ++
>  drivers/tty/serial/sc16is7xx_spi.c |  2 ++
>  4 files changed, 28 insertions(+)
> 
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index bf0065d1c8e9..53bfb603b03c 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -19,6 +19,7 @@
>  #include <linux/kthread.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> +#include <linux/of_gpio.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/sched.h>
> @@ -1467,6 +1468,27 @@ static const struct serial_rs485 sc16is7xx_rs485_supported = {
>  	.delay_rts_after_send = 1,	/* Not supported but keep returning -EINVAL */
>  };
>  
> +void sc16is7xx_setup_reset_pin(struct device *dev)

Potentially rename to sc16is7xx_reset() based on my comments below
about software reset.

> +{
> +	struct device_node *np = dev->of_node;
> +	int reset_gpio, err;
> +
> +	reset_gpio = of_get_named_gpio(np, "reset-gpios", 0);

Maybe use devm_gpiod_get_optional() to simplify and avoid OF-specific
code.

> +	if (!gpio_is_valid(reset_gpio))
> +		return;
> +
> +	err = devm_gpio_request_one(dev, reset_gpio, GPIOF_OUT_INIT_LOW,
> +				    "sc16is7xx-reset");
> +	if (err) {
> +		dev_err(dev, "failed to request sc16is7xx-reset-gpios: %d\n", err);
> +		return;

When this error happens, you return no error to the calling function,
why? If you specify a reset GPIO in your device tree, and you cannot use
it, seems like an error worth reporting.

> +	}
> +
> +	/* Deassert the reset pin */

Do you respect the manufacturer's minimum reset pulse width? The
datasheet states that its 3 us, so maybe add a delay before deassertion.

> +	gpio_set_value_cansleep(reset_gpio, 1);
> +}
> +EXPORT_SYMBOL_GPL(sc16is7xx_setup_reset_pin);
> +
>  int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
>  		    struct regmap *regmaps[], int irq)
>  {
> diff --git a/drivers/tty/serial/sc16is7xx.h b/drivers/tty/serial/sc16is7xx.h
> index afb784eaee45..f4ae114cc41a 100644
> --- a/drivers/tty/serial/sc16is7xx.h
> +++ b/drivers/tty/serial/sc16is7xx.h
> @@ -33,6 +33,8 @@ const char *sc16is7xx_regmap_name(u8 port_id);
>  
>  unsigned int sc16is7xx_regmap_port_mask(unsigned int port_id);
>  
> +void sc16is7xx_setup_reset_pin(struct device *dev);
> +
>  int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
>  		    struct regmap *regmaps[], int irq);
>  
> diff --git a/drivers/tty/serial/sc16is7xx_i2c.c b/drivers/tty/serial/sc16is7xx_i2c.c
> index 3ed47c306d85..9833c3b935c2 100644
> --- a/drivers/tty/serial/sc16is7xx_i2c.c
> +++ b/drivers/tty/serial/sc16is7xx_i2c.c
> @@ -21,6 +21,8 @@ static int sc16is7xx_i2c_probe(struct i2c_client *i2c)
>  	if (!devtype)
>  		return dev_err_probe(&i2c->dev, -ENODEV, "Failed to match device\n");
>  
> +	sc16is7xx_setup_reset_pin(&i2c->dev);

Move this call inside sc16is7xx_probe() function, since it is common to
both i2c and spi interfaces. Also, you will see in sc16is7xx_probe()
that we already issue a software reset. If you
specify a hardware reset pin, then you shouldn't issue the software
reset.

> +
>  	memcpy(&regcfg, &sc16is7xx_regcfg, sizeof(struct regmap_config));
>  
>  	for (i = 0; i < devtype->nr_uart; i++) {
> diff --git a/drivers/tty/serial/sc16is7xx_spi.c b/drivers/tty/serial/sc16is7xx_spi.c
> index 73df36f8a7fd..ce38561faaf0 100644
> --- a/drivers/tty/serial/sc16is7xx_spi.c
> +++ b/drivers/tty/serial/sc16is7xx_spi.c
> @@ -38,6 +38,8 @@ static int sc16is7xx_spi_probe(struct spi_device *spi)
>  	if (!devtype)
>  		return dev_err_probe(&spi->dev, -ENODEV, "Failed to match device\n");
>  
> +	sc16is7xx_setup_reset_pin(&spi->dev);
> +
>  	memcpy(&regcfg, &sc16is7xx_regcfg, sizeof(struct regmap_config));
>  
>  	for (i = 0; i < devtype->nr_uart; i++) {
> -- 
> 2.34.1
> 
> 
> 


-- 
Hugo Villeneuve

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios
  2024-06-03 12:37 [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios Hui Wang
  2024-06-03 12:37 ` [PATCH 2/2] serial: sc16is7xx: setup reset pin if it is defined in device tree Hui Wang
@ 2024-06-03 13:18 ` Rob Herring
  2024-06-04 10:35   ` Hui Wang
  2024-06-04  6:54 ` Krzysztof Kozlowski
  2 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2024-06-03 13:18 UTC (permalink / raw)
  To: Hui Wang; +Cc: linux-serial, devicetree, gregkh, jirislaby, hvilleneuve

On Mon, Jun 03, 2024 at 08:37:09PM +0800, Hui Wang wrote:
> In some designs, the chip reset pin is connected to a gpio, this
> gpio needs to be set correctly before probing the driver, so adding
> a reset-gpios in the device tree.
> 
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> index 5dec15b7e7c3..62aff6e034cb 100644
> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
> @@ -28,6 +28,9 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  reset-gpios:
> +    maxItems: 1
> +
>    clock-frequency:
>      description:
>        When there is no clock provider visible to the platform, this
> @@ -120,6 +123,7 @@ examples:
>              compatible = "nxp,sc16is752";
>              reg = <0x54>;
>              clocks = <&clk20m>;
> +            reset-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;

Missing the header for the define.

Test your bindings before sending.

>              interrupt-parent = <&gpio3>;
>              interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
>              nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios
  2024-06-03 12:37 [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios Hui Wang
  2024-06-03 12:37 ` [PATCH 2/2] serial: sc16is7xx: setup reset pin if it is defined in device tree Hui Wang
  2024-06-03 13:18 ` [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios Rob Herring
@ 2024-06-04  6:54 ` Krzysztof Kozlowski
  2024-06-04 10:35   ` Hui Wang
  2 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-06-04  6:54 UTC (permalink / raw)
  To: Hui Wang, linux-serial, devicetree, gregkh; +Cc: jirislaby, hvilleneuve

On 03/06/2024 14:37, Hui Wang wrote:
> In some designs, the chip reset pin is connected to a gpio, this
> gpio needs to be set correctly before probing the driver, so adding
> a reset-gpios in the device tree.
> 
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 4 ++++
>  1 file changed, 4 insertions(+)

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline), work on fork of kernel
(don't, instead use mainline) or you ignore some maintainers (really
don't). Just use b4 and everything should be fine, although remember
about `b4 prep --auto-to-cc` if you added new patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] serial: sc16is7xx: setup reset pin if it is defined in device tree
  2024-06-03 13:11   ` Hugo Villeneuve
@ 2024-06-04 10:34     ` Hui Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Hui Wang @ 2024-06-04 10:34 UTC (permalink / raw)
  To: Hugo Villeneuve; +Cc: linux-serial, devicetree, gregkh, jirislaby, hvilleneuve

Hi Hugo,

Will address all your comment in the v2.

Thanks,

Hui.

On 6/3/24 21:11, Hugo Villeneuve wrote:
> On Mon,  3 Jun 2024 20:37:10 +0800
> Hui Wang <hui.wang@canonical.com> wrote:
>
> Hi Hui,
>
>> Certain designs connect a gpio to the reset pin, and the reset pin
>> needs to be setup correctly before accessing the chip.
>>
>> Here adding a function to handle the reset pin. This change has no
>> impact if there is no reset_gpios defined in the device tree.
>>
>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>> ---
>>   drivers/tty/serial/sc16is7xx.c     | 22 ++++++++++++++++++++++
>>   drivers/tty/serial/sc16is7xx.h     |  2 ++
>>   drivers/tty/serial/sc16is7xx_i2c.c |  2 ++
>>   drivers/tty/serial/sc16is7xx_spi.c |  2 ++
>>   4 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
>> index bf0065d1c8e9..53bfb603b03c 100644
>> --- a/drivers/tty/serial/sc16is7xx.c
>> +++ b/drivers/tty/serial/sc16is7xx.c
>> @@ -19,6 +19,7 @@
>>   #include <linux/kthread.h>
>>   #include <linux/mod_devicetable.h>
>>   #include <linux/module.h>
>> +#include <linux/of_gpio.h>
>>   #include <linux/property.h>
>>   #include <linux/regmap.h>
>>   #include <linux/sched.h>
>> @@ -1467,6 +1468,27 @@ static const struct serial_rs485 sc16is7xx_rs485_supported = {
>>   	.delay_rts_after_send = 1,	/* Not supported but keep returning -EINVAL */
>>   };
>>   
>> +void sc16is7xx_setup_reset_pin(struct device *dev)
> Potentially rename to sc16is7xx_reset() based on my comments below
> about software reset.
>
>> +{
>> +	struct device_node *np = dev->of_node;
>> +	int reset_gpio, err;
>> +
>> +	reset_gpio = of_get_named_gpio(np, "reset-gpios", 0);
> Maybe use devm_gpiod_get_optional() to simplify and avoid OF-specific
> code.
>
>> +	if (!gpio_is_valid(reset_gpio))
>> +		return;
>> +
>> +	err = devm_gpio_request_one(dev, reset_gpio, GPIOF_OUT_INIT_LOW,
>> +				    "sc16is7xx-reset");
>> +	if (err) {
>> +		dev_err(dev, "failed to request sc16is7xx-reset-gpios: %d\n", err);
>> +		return;
> When this error happens, you return no error to the calling function,
> why? If you specify a reset GPIO in your device tree, and you cannot use
> it, seems like an error worth reporting.
>
>> +	}
>> +
>> +	/* Deassert the reset pin */
> Do you respect the manufacturer's minimum reset pulse width? The
> datasheet states that its 3 us, so maybe add a delay before deassertion.
>
>> +	gpio_set_value_cansleep(reset_gpio, 1);
>> +}
>> +EXPORT_SYMBOL_GPL(sc16is7xx_setup_reset_pin);
>> +
>>   int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
>>   		    struct regmap *regmaps[], int irq)
>>   {
>> diff --git a/drivers/tty/serial/sc16is7xx.h b/drivers/tty/serial/sc16is7xx.h
>> index afb784eaee45..f4ae114cc41a 100644
>> --- a/drivers/tty/serial/sc16is7xx.h
>> +++ b/drivers/tty/serial/sc16is7xx.h
>> @@ -33,6 +33,8 @@ const char *sc16is7xx_regmap_name(u8 port_id);
>>   
>>   unsigned int sc16is7xx_regmap_port_mask(unsigned int port_id);
>>   
>> +void sc16is7xx_setup_reset_pin(struct device *dev);
>> +
>>   int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
>>   		    struct regmap *regmaps[], int irq);
>>   
>> diff --git a/drivers/tty/serial/sc16is7xx_i2c.c b/drivers/tty/serial/sc16is7xx_i2c.c
>> index 3ed47c306d85..9833c3b935c2 100644
>> --- a/drivers/tty/serial/sc16is7xx_i2c.c
>> +++ b/drivers/tty/serial/sc16is7xx_i2c.c
>> @@ -21,6 +21,8 @@ static int sc16is7xx_i2c_probe(struct i2c_client *i2c)
>>   	if (!devtype)
>>   		return dev_err_probe(&i2c->dev, -ENODEV, "Failed to match device\n");
>>   
>> +	sc16is7xx_setup_reset_pin(&i2c->dev);
> Move this call inside sc16is7xx_probe() function, since it is common to
> both i2c and spi interfaces. Also, you will see in sc16is7xx_probe()
> that we already issue a software reset. If you
> specify a hardware reset pin, then you shouldn't issue the software
> reset.
>
>> +
>>   	memcpy(&regcfg, &sc16is7xx_regcfg, sizeof(struct regmap_config));
>>   
>>   	for (i = 0; i < devtype->nr_uart; i++) {
>> diff --git a/drivers/tty/serial/sc16is7xx_spi.c b/drivers/tty/serial/sc16is7xx_spi.c
>> index 73df36f8a7fd..ce38561faaf0 100644
>> --- a/drivers/tty/serial/sc16is7xx_spi.c
>> +++ b/drivers/tty/serial/sc16is7xx_spi.c
>> @@ -38,6 +38,8 @@ static int sc16is7xx_spi_probe(struct spi_device *spi)
>>   	if (!devtype)
>>   		return dev_err_probe(&spi->dev, -ENODEV, "Failed to match device\n");
>>   
>> +	sc16is7xx_setup_reset_pin(&spi->dev);
>> +
>>   	memcpy(&regcfg, &sc16is7xx_regcfg, sizeof(struct regmap_config));
>>   
>>   	for (i = 0; i < devtype->nr_uart; i++) {
>> -- 
>> 2.34.1
>>
>>
>>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios
  2024-06-03 13:18 ` [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios Rob Herring
@ 2024-06-04 10:35   ` Hui Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Hui Wang @ 2024-06-04 10:35 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-serial, devicetree, gregkh, jirislaby, hvilleneuve


On 6/3/24 21:18, Rob Herring wrote:
> On Mon, Jun 03, 2024 at 08:37:09PM +0800, Hui Wang wrote:
>> In some designs, the chip reset pin is connected to a gpio, this
>> gpio needs to be set correctly before probing the driver, so adding
>> a reset-gpios in the device tree.
>>
>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>> ---
>>   Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
>> index 5dec15b7e7c3..62aff6e034cb 100644
>> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
>> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
>> @@ -28,6 +28,9 @@ properties:
>>     clocks:
>>       maxItems: 1
>>   
>> +  reset-gpios:
>> +    maxItems: 1
>> +
>>     clock-frequency:
>>       description:
>>         When there is no clock provider visible to the platform, this
>> @@ -120,6 +123,7 @@ examples:
>>               compatible = "nxp,sc16is752";
>>               reg = <0x54>;
>>               clocks = <&clk20m>;
>> +            reset-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
> Missing the header for the define.
>
> Test your bindings before sending.

OK, got it.

Thanks.

>
>>               interrupt-parent = <&gpio3>;
>>               interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
>>               nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
>> -- 
>> 2.34.1
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios
  2024-06-04  6:54 ` Krzysztof Kozlowski
@ 2024-06-04 10:35   ` Hui Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Hui Wang @ 2024-06-04 10:35 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-serial, devicetree, gregkh
  Cc: jirislaby, hvilleneuve


On 6/4/24 14:54, Krzysztof Kozlowski wrote:
> On 03/06/2024 14:37, Hui Wang wrote:
>> In some designs, the chip reset pin is connected to a gpio, this
>> gpio needs to be set correctly before probing the driver, so adding
>> a reset-gpios in the device tree.
>>
>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>> ---
>>   Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 4 ++++
>>   1 file changed, 4 insertions(+)
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline), work on fork of kernel
> (don't, instead use mainline) or you ignore some maintainers (really
> don't). Just use b4 and everything should be fine, although remember
> about `b4 prep --auto-to-cc` if you added new patches to the patchset.
>
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time.
>
> Please kindly resend and include all necessary To/Cc entries.
>
> Best regards,
> Krzysztof
>
Got it. thanks.



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-06-04 10:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-03 12:37 [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios Hui Wang
2024-06-03 12:37 ` [PATCH 2/2] serial: sc16is7xx: setup reset pin if it is defined in device tree Hui Wang
2024-06-03 13:11   ` Hugo Villeneuve
2024-06-04 10:34     ` Hui Wang
2024-06-03 13:18 ` [PATCH 1/2] dt-bindings: serial: sc16is7xx: add reset-gpios Rob Herring
2024-06-04 10:35   ` Hui Wang
2024-06-04  6:54 ` Krzysztof Kozlowski
2024-06-04 10:35   ` Hui Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).