diff mbox series

[3/6] dt-bindings: serdev: ngsm: Add binding for GNSS child node

Message ID 20200512214713.40501-4-tony@atomide.com (mailing list archive)
State New, archived
Headers show
Series n_gsm serdev support and GNSS driver for droid4 | expand

Commit Message

Tony Lindgren May 12, 2020, 9:47 p.m. UTC
For motorola modem case, we may have a GNSS device on channel 4.
Let's add that to the binding and example.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 .../devicetree/bindings/serdev/serdev-ngsm.yaml          | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Pavel Machek May 13, 2020, 7:26 p.m. UTC | #1
On Tue 2020-05-12 14:47:10, Tony Lindgren wrote:
> For motorola modem case, we may have a GNSS device on channel 4.
> Let's add that to the binding and example.
> 
> Signed-off-by: Tony Lindgren <tony@atomide.com>

Reviewed-by: Pavel Machek <pavel@ucw.cz>

BTW it looks like Alan's email address no longer works.

<gnomes@lxorguk.ukuu.org.uk>: host mail.llwyncelyn.cymru[82.70.14.225] said:
    550 5.1.1 <gnomes@lxorguk.ukuu.org.uk>... User unknown (in reply to RCPT TO

Best regards,
								Pavel
Rob Herring (Arm) May 27, 2020, 7:28 p.m. UTC | #2
On Tue, May 12, 2020 at 02:47:10PM -0700, Tony Lindgren wrote:
> For motorola modem case, we may have a GNSS device on channel 4.
> Let's add that to the binding and example.
> 
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  .../devicetree/bindings/serdev/serdev-ngsm.yaml          | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> --- a/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> +++ b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> @@ -42,6 +42,10 @@ allOf:
>            description: Name of the USB PHY
>            const: usb
>  
> +        compatible:
> +          description: GNSS receiver
> +          const: motorola,mapphone-mdm6600-gnss

I'm not sure how this isn't failing on the example because it is wrong.

You're saying this compatible belongs at the same level as 
phys/phy-names, but that would be the parent which already has a 
compatible. You have to define a child node property (gnss@4) and have 
'compatible' under it. At that point, this schema becomes very much 
Motorola specific.

> +
>        required:
>          - phys
>          - phy-names
> @@ -61,4 +65,9 @@ examples:
>        phy-names = "usb";
>        #address-cells = <1>;
>        #size-cells = <0>;
> +
> +      gnss@4 {
> +         compatible = "motorola,mapphone-mdm6600-gnss";
> +         reg = <4>;
> +      };
>      };
> -- 
> 2.26.2
Johan Hovold May 28, 2020, 9:51 a.m. UTC | #3
On Wed, May 27, 2020 at 01:28:17PM -0600, Rob Herring wrote:
> On Tue, May 12, 2020 at 02:47:10PM -0700, Tony Lindgren wrote:
> > For motorola modem case, we may have a GNSS device on channel 4.
> > Let's add that to the binding and example.
> > 
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > ---
> >  .../devicetree/bindings/serdev/serdev-ngsm.yaml          | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> > --- a/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> > +++ b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> > @@ -42,6 +42,10 @@ allOf:
> >            description: Name of the USB PHY
> >            const: usb
> >  
> > +        compatible:
> > +          description: GNSS receiver
> > +          const: motorola,mapphone-mdm6600-gnss
> 
> I'm not sure how this isn't failing on the example because it is wrong.
> 
> You're saying this compatible belongs at the same level as 
> phys/phy-names, but that would be the parent which already has a 
> compatible. You have to define a child node property (gnss@4) and have 
> 'compatible' under it. At that point, this schema becomes very much 
> Motorola specific.
> 
> > +
> >        required:
> >          - phys
> >          - phy-names
> > @@ -61,4 +65,9 @@ examples:
> >        phy-names = "usb";
> >        #address-cells = <1>;
> >        #size-cells = <0>;
> > +
> > +      gnss@4 {
> > +         compatible = "motorola,mapphone-mdm6600-gnss";
> > +         reg = <4>;
> > +      };
> >      };
> > -- 
> > 2.26.2

And since we're describing a mux, I think you need nodes for the virtual
ports rather than a reg property in what should be a serial client. That
is something like

	serial@nnn {
		modem {
			compatible = "etsi,ts27001-mux";

			serial@4 {
				compatible = "etsi,ts27001-serial";
				reg = <4>;

				gnss {
					compatible = "motorola,motmdm-gnss";
				};
			};
		};
	};

This way you can actually use serdev for the client drivers (e.g. for
gnss), and those drivers also be used for non-muxed ports if needed
(e.g. over USB).

Johan
Pavel Machek March 5, 2021, 10:46 a.m. UTC | #4
Hi!

> > > For motorola modem case, we may have a GNSS device on channel 4.
> > > Let's add that to the binding and example.
> > > 
> > > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > > ---
> > >  .../devicetree/bindings/serdev/serdev-ngsm.yaml          | 9 +++++++++
> > >  1 file changed, 9 insertions(+)

> 
> And since we're describing a mux, I think you need nodes for the virtual
> ports rather than a reg property in what should be a serial client. That
> is something like
> 
> 	serial@nnn {
> 		modem {
> 			compatible = "etsi,ts27001-mux";
> 
> 			serial@4 {
> 				compatible = "etsi,ts27001-serial";
> 				reg = <4>;
> 
> 				gnss {
> 					compatible = "motorola,motmdm-gnss";
> 				};
> 			};
> 		};
> 	};
> 
> This way you can actually use serdev for the client drivers (e.g. for
> gnss), and those drivers also be used for non-muxed ports if needed
> (e.g. over USB).

I have done changes you requested, and then hit "serdev is busy
because it can have at most one child" limit in the code. You have
pretty clean driver in your inbox, and no reply. No help with serdev
core limitations, either. Can you start to communicate?

Best regards,
								Pavel
Johan Hovold March 5, 2021, 10:52 a.m. UTC | #5
On Fri, Mar 05, 2021 at 11:46:35AM +0100, Pavel Machek wrote:
> Hi!
> 
> > > > For motorola modem case, we may have a GNSS device on channel 4.
> > > > Let's add that to the binding and example.
> > > > 
> > > > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > > > ---
> > > >  .../devicetree/bindings/serdev/serdev-ngsm.yaml          | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> 
> > 
> > And since we're describing a mux, I think you need nodes for the virtual
> > ports rather than a reg property in what should be a serial client. That
> > is something like
> > 
> > 	serial@nnn {
> > 		modem {
> > 			compatible = "etsi,ts27001-mux";
> > 
> > 			serial@4 {
> > 				compatible = "etsi,ts27001-serial";
> > 				reg = <4>;
> > 
> > 				gnss {
> > 					compatible = "motorola,motmdm-gnss";
> > 				};
> > 			};
> > 		};
> > 	};
> > 
> > This way you can actually use serdev for the client drivers (e.g. for
> > gnss), and those drivers also be used for non-muxed ports if needed
> > (e.g. over USB).
> 
> I have done changes you requested, and then hit "serdev is busy
> because it can have at most one child" limit in the code. You have
> pretty clean driver in your inbox, and no reply. No help with serdev
> core limitations, either. Can you start to communicate?

It's on my list, but time is limited.

Johan
Pavel Machek March 24, 2021, 1:09 a.m. UTC | #6
On Fri 2021-03-05 11:52:38, Johan Hovold wrote:
> On Fri, Mar 05, 2021 at 11:46:35AM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > > > For motorola modem case, we may have a GNSS device on channel 4.
> > > > > Let's add that to the binding and example.
> > > > > 
> > > > > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > > > > ---
> > > > >  .../devicetree/bindings/serdev/serdev-ngsm.yaml          | 9 +++++++++
> > > > >  1 file changed, 9 insertions(+)
> > 
> > > 
> > > And since we're describing a mux, I think you need nodes for the virtual
> > > ports rather than a reg property in what should be a serial client. That
> > > is something like
> > > 
> > > 	serial@nnn {
> > > 		modem {
> > > 			compatible = "etsi,ts27001-mux";
> > > 
> > > 			serial@4 {
> > > 				compatible = "etsi,ts27001-serial";
> > > 				reg = <4>;
> > > 
> > > 				gnss {
> > > 					compatible = "motorola,motmdm-gnss";
> > > 				};
> > > 			};
> > > 		};
> > > 	};
> > > 
> > > This way you can actually use serdev for the client drivers (e.g. for
> > > gnss), and those drivers also be used for non-muxed ports if needed
> > > (e.g. over USB).
> > 
> > I have done changes you requested, and then hit "serdev is busy
> > because it can have at most one child" limit in the code. You have
> > pretty clean driver in your inbox, and no reply. No help with serdev
> > core limitations, either. Can you start to communicate?
> 
> It's on my list, but time is limited.

Everyone's time is limited. Do you have any time estimates?
								Pavel
Johan Hovold April 1, 2021, 9:43 a.m. UTC | #7
On Fri, Mar 05, 2021 at 11:46:35AM +0100, Pavel Machek wrote:
> Hi!
> 
> > > > For motorola modem case, we may have a GNSS device on channel 4.
> > > > Let's add that to the binding and example.
> > > > 
> > > > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > > > ---
> > > >  .../devicetree/bindings/serdev/serdev-ngsm.yaml          | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> 
> > 
> > And since we're describing a mux, I think you need nodes for the virtual
> > ports rather than a reg property in what should be a serial client. That
> > is something like
> > 
> > 	serial@nnn {
> > 		modem {
> > 			compatible = "etsi,ts27001-mux";
> > 
> > 			serial@4 {
> > 				compatible = "etsi,ts27001-serial";
> > 				reg = <4>;
> > 
> > 				gnss {
> > 					compatible = "motorola,motmdm-gnss";
> > 				};
> > 			};
> > 		};
> > 	};
> > 
> > This way you can actually use serdev for the client drivers (e.g. for
> > gnss), and those drivers also be used for non-muxed ports if needed
> > (e.g. over USB).
> 
> I have done changes you requested, and then hit "serdev is busy
> because it can have at most one child" limit in the code. You have
> pretty clean driver in your inbox, and no reply. No help with serdev
> core limitations, either. Can you start to communicate?

Heh, look at the devicetree example above that I gave you back in March.
It has the virtual serial ports described ("serial@4"), which you were
missing.

Johan
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
--- a/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
+++ b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
@@ -42,6 +42,10 @@  allOf:
           description: Name of the USB PHY
           const: usb
 
+        compatible:
+          description: GNSS receiver
+          const: motorola,mapphone-mdm6600-gnss
+
       required:
         - phys
         - phy-names
@@ -61,4 +65,9 @@  examples:
       phy-names = "usb";
       #address-cells = <1>;
       #size-cells = <0>;
+
+      gnss@4 {
+         compatible = "motorola,mapphone-mdm6600-gnss";
+         reg = <4>;
+      };
     };