linux-kernel - Re: [PATCH 10/12] serial: 8250_dw: Add Sophgo SG2042 support

Open Source and information security mailing list archives

Message-ID: <CAJM55Z-GMm5Hmk7-QWt8Prvp7qyFzOrQVjOjB7jfoX-oiT_C1A@mail.gmail.com>
Date:   Fri, 15 Sep 2023 03:02:45 -0700
From:   Emil Renner Berthing <emil.renner.berthing@...onical.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Wang Chen <unicornxw@...il.com>,
        linux-riscv@...ts.infradead.org, conor@...nel.org,
        aou@...s.berkeley.edu, krzysztof.kozlowski+dt@...aro.org,
        palmer@...belt.com, paul.walmsley@...ive.com, robh+dt@...nel.org
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        jszhang@...nel.org, guoren@...nel.org, chao.wei@...hgo.com,
        xiaoguang.xing@...hgo.com,
        Emil Renner Berthing <emil.renner.berthing@...onical.com>
Subject: Re: [PATCH 10/12] serial: 8250_dw: Add Sophgo SG2042 support

Krzysztof Kozlowski wrote:
> On 15/09/2023 09:25, Wang Chen wrote:
> > From: Emil Renner Berthing <emil.renner.berthing@...onical.com>
> >
> > Add quirk to skip setting the input clock rate for the uarts on the
> > Sophgo SG2042 SoC similar to the StarFive JH7100.
> >
> > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@...onical.com>
>
> Missing SoB.
>
> > ---
> >  drivers/tty/serial/8250/8250_dw.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > index f4cafca1a7da..6c344877a07f 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -770,7 +770,7 @@ static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {
> >  	.quirks = DW_UART_QUIRK_IS_DMA_FC,
> >  };
> >
> > -static const struct dw8250_platform_data dw8250_starfive_jh7100_data = {
> > +static const struct dw8250_platform_data dw8250_skip_set_rate_data = {
>
> Why? What is wrong with old name?
>
> >  	.usr_reg = DW_UART_USR,
> >  	.quirks = DW_UART_QUIRK_SKIP_SET_RATE,
> >  };
> > @@ -780,7 +780,8 @@ static const struct of_device_id dw8250_of_match[] = {
> >  	{ .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
> >  	{ .compatible = "marvell,armada-38x-uart", .data = &dw8250_armada_38x_data },
> >  	{ .compatible = "renesas,rzn1-uart", .data = &dw8250_renesas_rzn1_data },
> > -	{ .compatible = "starfive,jh7100-uart", .data = &dw8250_starfive_jh7100_data },
> > +	{ .compatible = "sophgo,sg2042-uart", .data = &dw8250_skip_set_rate_data },
> > +	{ .compatible = "starfive,jh7100-uart", .data = &dw8250_skip_set_rate_data },
>
> So devices are fully compatible? Then use compatibility and drop this
> patch entirely.

I'm fine with this, but these are two different companies and SoCs that just
happens to have both implemented the Designware UART with an inflexible input
clock. So if fx. a real quirk is found on the JH7110 then we'd need to either
change the compatible on an unrelated SoC or change compatible on the JH7110 to
something like "starfive,jh7100-uart-with-quirk" and "starfive,jh7100-uart" will
forever be a quirky way to spell "dw8250 with inflexible input clock".
Is that how device trees are supposed to work?

/Emil
>
> Best regards,
> Krzysztof
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives