Re: [PATCH] sh-sci: add R8A7743/5 support

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Wednesday 05 Oct 2016 12:03:24 Geert Uytterhoeven wrote:
> Hi Laurent,
> 
> On Wed, Oct 5, 2016 at 11:55 AM, Laurent Pinchart
> 
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Friday 30 Sep 2016 10:38:48 Geert Uytterhoeven wrote:
> >> On Thu, Sep 29, 2016 at 11:37 PM, Sergei Shtylyov wrote:
> >> > Renesas  RZ/G SoC also have the SCIF, SCIFA, SCIFB, and HSCIF ports.
> >> > Document RZ/G1[ME] (also known as R8A774[35]) SoC bindings along with
> >> > the RZ/G family bindings.  The driver itself also needs to recognize
> >> > the latter binding for the SCIF ports, so teach it...
> >> > 
> >> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >> > 
> >> > ---
> >> > This patch is against the 'tty-next' branch of GregKH's 'tty.git' repo.
> >> > 
> >> >  Documentation/devicetree/bindings/serial/renesas,sci-serial.txt |   12
> >> >  ++++++++++
> >> >  drivers/tty/serial/sh-sci.c                                     |    3
> >> >  ++
> >> >  2 files changed, 15 insertions(+)
> >> > 
> >> > Index:
> >> > tty/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> >> > ===================================================================
> >> > ---
> >> > tty.orig/Documentation/devicetree/bindings/serial/renesas,sci-serial.tx
> >> > t
> >> > +++
> >> > tty/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt>
> >> > 
> >> > @@ -9,6 +9,14 @@ Required properties:
> >> >      - "renesas,scifb-r8a73a4" for R8A73A4 (R-Mobile APE6) SCIFB
> >> >      compatible UART. - "renesas,scifa-r8a7740" for R8A7740 (R-Mobile
> >> >      A1)
> >> >      SCIFA compatible UART. - "renesas,scifb-r8a7740" for R8A7740
> >> >      (R-Mobile A1) SCIFB compatible UART.>
> >> > 
> >> > +    - "renesas,scif-r8a7743" for R8A7743 (RZ/G1M) SCIF compatible
> >> > UART.
> >> > +    - "renesas,scifa-r8a7743" for R8A7743 (RZ/G1M) SCIFA compatible
> >> > UART.
> >> > +    - "renesas,scifb-r8a7743" for R8A7743 (RZ/G1M) SCIFB compatible
> >> > UART.
> >> > +    - "renesas,hscif-r8a7743" for R8A7743 (RZ/G1M) HSCIF compatible
> >> > UART.
> >> > +    - "renesas,scif-r8a7745" for R8A7745 (RZ/G1E) SCIF compatible
> >> > UART.
> >> > +    - "renesas,scifa-r8a7745" for R8A7745 (RZ/G1E) SCIFA compatible
> >> > UART.
> >> > +    - "renesas,scifb-r8a7745" for R8A7745 (RZ/G1E) SCIFB compatible
> >> > UART.
> >> > +    - "renesas,hscif-r8a7745" for R8A7745 (RZ/G1E) HSCIF compatible
> >> > UART.
> > 
> > Are we *really* going to add four compat string for each RZ/G* SoC ? We're
> > reaching the limit of sanity...
> 
> Soon we can use soc_device_match(), if we ever need to handle r8a7743 or
> r8a7745 quirks?

We could, but I believe usage of soc_device_match() should be kept minimal, 
otherwise we could remove all compat strings but one and handle all 
differences in the driver with soc_device_match(). That's a topic I'd like to 
discuss face to face in Berlin. The recent new compatible strings show we have 
a problem, and we need to agree on how to address it.

> Declaring it is compatible with renesas,rcar-gen2-scif is still good/needed
> for the BRG (Baud Rate Generator for External Clock), which is not present
> on e.g. RZ/A1H.

Given that the RZ/G1[ME] SCIFs seem to be compatible with the Gen2 SCIFs using 
the renesas,rcar-gen2-scif compat string seems fine to me.

-- 
Regards,

Laurent Pinchart