diff mbox series

[v4,3/6] dt-bindings: serial: add binding for rs485 rx-enable state when rs485 is disabled

Message ID 20240126-dev-rx-enable-v4-3-45aaf4d96328@theobroma-systems.com
State New
Headers
Series serial: 8250: Add support for rs485 half/full duplex on puma/ringneck-haikou |

Commit Message

Farouk Bouabid Jan. 26, 2024, 2:55 p.m. UTC 
  RS485 can have a receiver-enable gpio (rx-enable-gpios). When rs485 is
enabled, this gpio, if provided, must be driven active while receiving.
However when RS485 is disabled this gpio should not have an undefined
state. In that case, as DE and RE pins can be connected both to this gpio,
if its state is not properly defined, can cause unexpected transceiver
behavior.
This binding depend on rx-enable-gpios to be implemented.

Signed-off-by: Farouk Bouabid <farouk.bouabid@theobroma-systems.com>
---
 Documentation/devicetree/bindings/serial/rs485.yaml | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Conor Dooley Jan. 28, 2024, 5:38 p.m. UTC | #1 
On Fri, Jan 26, 2024 at 03:55:12PM +0100, Farouk Bouabid wrote:
> RS485 can have a receiver-enable gpio (rx-enable-gpios). When rs485 is
> enabled, this gpio, if provided, must be driven active while receiving.
> However when RS485 is disabled this gpio should not have an undefined
> state. In that case, as DE and RE pins can be connected both to this gpio,
> if its state is not properly defined, can cause unexpected transceiver
> behavior.
> This binding depend on rx-enable-gpios to be implemented.

Why do you need a dedicated property for this when there exists a device
specific compatible for the uart on both of the affected rockchip
systems?

Thanks,
Conor.

> 
> Signed-off-by: Farouk Bouabid <farouk.bouabid@theobroma-systems.com>
> ---
>  Documentation/devicetree/bindings/serial/rs485.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
> index b64577036b5c..4c79dfaaf460 100644
> --- a/Documentation/devicetree/bindings/serial/rs485.yaml
> +++ b/Documentation/devicetree/bindings/serial/rs485.yaml
> @@ -55,6 +55,11 @@ properties:
>      description: GPIO to handle a separate RS485 receive enable signal
>      maxItems: 1
>  
> +  rs485-rx-enable-inactive-when-rs485-disabled:
> +    description: rx-enable GPIO is not active when RS485 is disabled. If missing, active-state
> +      is assumed.
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
>    rs485-term-gpios:
>      description: GPIO pin to enable RS485 bus termination.
>      maxItems: 1
> 
> -- 
> 2.34.1
>
Quentin Schulz Jan. 29, 2024, 12:26 p.m. UTC | #2 
Hi Conor,

On 1/28/24 18:38, Conor Dooley wrote:
> On Fri, Jan 26, 2024 at 03:55:12PM +0100, Farouk Bouabid wrote:
>> RS485 can have a receiver-enable gpio (rx-enable-gpios). When rs485 is
>> enabled, this gpio, if provided, must be driven active while receiving.
>> However when RS485 is disabled this gpio should not have an undefined
>> state. In that case, as DE and RE pins can be connected both to this gpio,
>> if its state is not properly defined, can cause unexpected transceiver
>> behavior.
>> This binding depend on rx-enable-gpios to be implemented.
> 
> Why do you need a dedicated property for this when there exists a device
> specific compatible for the uart on both of the affected rockchip
> systems?
> 

This has nothing to do with Rockchip's IP but the HW design of our 
carrierboard, so using the "rockchip,px30-uart" for that (which I assume 
is what was suggested here?) is incorrect since it'll also apply to 
PX30, RK3399 and RK3588-based Q7 SoCs we manufacture.

Did I understand the suggestion correctly?

Cheers,
Quentin
Conor Dooley Jan. 29, 2024, 5:22 p.m. UTC | #3 
On Mon, Jan 29, 2024 at 01:26:51PM +0100, Quentin Schulz wrote:
> Hi Conor,
> 
> On 1/28/24 18:38, Conor Dooley wrote:
> > On Fri, Jan 26, 2024 at 03:55:12PM +0100, Farouk Bouabid wrote:
> > > RS485 can have a receiver-enable gpio (rx-enable-gpios). When rs485 is
> > > enabled, this gpio, if provided, must be driven active while receiving.
> > > However when RS485 is disabled this gpio should not have an undefined
> > > state. In that case, as DE and RE pins can be connected both to this gpio,
> > > if its state is not properly defined, can cause unexpected transceiver
> > > behavior.
> > > This binding depend on rx-enable-gpios to be implemented.
> > 
> > Why do you need a dedicated property for this when there exists a device
> > specific compatible for the uart on both of the affected rockchip
> > systems?
> > 
> 
> This has nothing to do with Rockchip's IP but the HW design of our
> carrierboard, so using the "rockchip,px30-uart" for that (which I assume is
> what was suggested here?) is incorrect since it'll also apply to PX30,
> RK3399 and RK3588-based Q7 SoCs we manufacture.
> 
> Did I understand the suggestion correctly?

Yes you did. That explanation for not being able to use the compatibles
makes sense. However, I can't give you an ack, because reading the
commit message gives me the same feeling as looking at this photo:
https://www.reddit.com/r/pics/comments/f8jyuz/nothing_in_this_image_is_identifiable/

Sorry,
Conor.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
index b64577036b5c..4c79dfaaf460 100644
--- a/Documentation/devicetree/bindings/serial/rs485.yaml
+++ b/Documentation/devicetree/bindings/serial/rs485.yaml
@@ -55,6 +55,11 @@  properties:
     description: GPIO to handle a separate RS485 receive enable signal
     maxItems: 1
 
+  rs485-rx-enable-inactive-when-rs485-disabled:
+    description: rx-enable GPIO is not active when RS485 is disabled. If missing, active-state
+      is assumed.
+    $ref: /schemas/types.yaml#/definitions/flag
+
   rs485-term-gpios:
     description: GPIO pin to enable RS485 bus termination.
     maxItems: 1