| Message ID | 20220311144814.21944-1-kernel@esmil.dk (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | serial: 8250_dw: Use device tree match data | expand |
On Fri, Mar 11, 2022 at 03:48:14PM +0100, Emil Renner Berthing wrote: > ..rather than multiple calls to of_device_is_compatible(). > For reference this is the patch I wrote for the StarFive JH7100 tree. > Feel free to use it or do something better as you see fit. > if (np) { > + unsigned long quirks = (unsigned long)of_device_get_match_data(p->dev); It can be done outside of the np check with device property APIs in use. Also it needs to use (uintptr_t) for better coverage. unsigned long quirks = (uintptr_t)device_get_match_data(p->dev); Or use data structure as driver_data. const struct ... *data = device_get_match_data(p->dev);
Hi Emil, kernel@esmil.dk wrote on Fri, 11 Mar 2022 15:48:14 +0100: > ..rather than multiple calls to of_device_is_compatible(). > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > --- > > Hi Miquel, > > > > > > --- a/drivers/tty/serial/8250/8250_dma.c > > > > > +++ b/drivers/tty/serial/8250/8250_dma.c =20 > > > > =20 > > > > > @@ -501,6 +589,8 @@ static int dw8250_probe(struct platform_device *p= > > dev) > > > > > data->msr_mask_off |=3D UART_MSR_TERI; > > > > > } > > > > > > > > > > + data->is_rzn1 =3D of_device_is_compatible(dev->of_node, "rene= > > sas,rzn1-uart"); =20 > > > > > > > > Explicit checks for compatible values are frowned upon if you have > > > > a match table. > > > > Please handle this through of_device.data, cfr. the various quirks. =20 > > >=20 > > > Oops, these are not yet upstream, but present in my tree due to including > > > support for StarLight, cfr. > > > https://github.com/esmil/linux/commits/visionfive/drivers/tty/serial/8250= > > /8250_dw.c > > > > Oh thanks for pointing it! Too bad that these quirks were not > > introduced inside a wider structure, I think it's always a must even if > > there is only one parameter there. Anyway, I'll introduce a wider > > specific structure and use it. > > For reference this is the patch I wrote for the StarFive JH7100 tree. > Feel free to use it or do something better as you see fit. Thanks for the pointers, I've fetched the three 8250_dw patches from your tree directly, and will build on top of them! Cheers, Miquèl > > /Emil > > drivers/tty/serial/8250/8250_dw.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c > index 1769808031c5..f564a019a7be 100644 > --- a/drivers/tty/serial/8250/8250_dw.c > +++ b/drivers/tty/serial/8250/8250_dw.c > @@ -37,6 +37,11 @@ > /* DesignWare specific register fields */ > #define DW_UART_MCR_SIRE BIT(6) > > +/* Quirks */ > +#define DW_UART_QUIRK_OCTEON BIT(0) > +#define DW_UART_QUIRK_ARMADA_38X BIT(1) > +#define DW_UART_QUIRK_SKIP_SET_RATE BIT(2) > + > struct dw8250_data { > struct dw8250_port_data data; > > @@ -389,6 +394,7 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data) > struct device_node *np = p->dev->of_node; > > if (np) { > + unsigned long quirks = (unsigned long)of_device_get_match_data(p->dev); > int id; > > /* get index of serial line, if found in DT aliases */ > @@ -396,7 +402,7 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data) > if (id >= 0) > p->line = id; > #ifdef CONFIG_64BIT > - if (of_device_is_compatible(np, "cavium,octeon-3860-uart")) { > + if (quirks & DW_UART_QUIRK_OCTEON) { > p->serial_in = dw8250_serial_inq; > p->serial_out = dw8250_serial_outq; > p->flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_FIXED_TYPE; > @@ -412,9 +418,9 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data) > p->serial_out = dw8250_serial_out32be; > } > > - if (of_device_is_compatible(np, "marvell,armada-38x-uart")) > + if (quirks & DW_UART_QUIRK_ARMADA_38X) > p->serial_out = dw8250_serial_out38x; > - if (of_device_is_compatible(np, "starfive,jh7100-uart")) > + if (quirks & DW_UART_QUIRK_SKIP_SET_RATE) > p->set_termios = dw8250_do_set_termios; > > } else if (acpi_dev_present("APMC0D08", NULL, -1)) { > @@ -695,10 +701,10 @@ static const struct dev_pm_ops dw8250_pm_ops = { > > static const struct of_device_id dw8250_of_match[] = { > { .compatible = "snps,dw-apb-uart" }, > - { .compatible = "cavium,octeon-3860-uart" }, > - { .compatible = "marvell,armada-38x-uart" }, > + { .compatible = "cavium,octeon-3860-uart", .data = (void *)DW_UART_QUIRK_OCTEON }, > + { .compatible = "marvell,armada-38x-uart", .data = (void *)DW_UART_QUIRK_ARMADA_38X }, > { .compatible = "renesas,rzn1-uart" }, > - { .compatible = "starfive,jh7100-uart" }, > + { .compatible = "starfive,jh7100-uart", .data = (void *)DW_UART_QUIRK_SKIP_SET_RATE }, > { /* Sentinel */ } > }; > MODULE_DEVICE_TABLE(of, dw8250_of_match); Thanks, Miquèl
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index 1769808031c5..f564a019a7be 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -37,6 +37,11 @@ /* DesignWare specific register fields */ #define DW_UART_MCR_SIRE BIT(6) +/* Quirks */ +#define DW_UART_QUIRK_OCTEON BIT(0) +#define DW_UART_QUIRK_ARMADA_38X BIT(1) +#define DW_UART_QUIRK_SKIP_SET_RATE BIT(2) + struct dw8250_data { struct dw8250_port_data data; @@ -389,6 +394,7 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data) struct device_node *np = p->dev->of_node; if (np) { + unsigned long quirks = (unsigned long)of_device_get_match_data(p->dev); int id; /* get index of serial line, if found in DT aliases */ @@ -396,7 +402,7 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data) if (id >= 0) p->line = id; #ifdef CONFIG_64BIT - if (of_device_is_compatible(np, "cavium,octeon-3860-uart")) { + if (quirks & DW_UART_QUIRK_OCTEON) { p->serial_in = dw8250_serial_inq; p->serial_out = dw8250_serial_outq; p->flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_FIXED_TYPE; @@ -412,9 +418,9 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data) p->serial_out = dw8250_serial_out32be; } - if (of_device_is_compatible(np, "marvell,armada-38x-uart")) + if (quirks & DW_UART_QUIRK_ARMADA_38X) p->serial_out = dw8250_serial_out38x; - if (of_device_is_compatible(np, "starfive,jh7100-uart")) + if (quirks & DW_UART_QUIRK_SKIP_SET_RATE) p->set_termios = dw8250_do_set_termios; } else if (acpi_dev_present("APMC0D08", NULL, -1)) { @@ -695,10 +701,10 @@ static const struct dev_pm_ops dw8250_pm_ops = { static const struct of_device_id dw8250_of_match[] = { { .compatible = "snps,dw-apb-uart" }, - { .compatible = "cavium,octeon-3860-uart" }, - { .compatible = "marvell,armada-38x-uart" }, + { .compatible = "cavium,octeon-3860-uart", .data = (void *)DW_UART_QUIRK_OCTEON }, + { .compatible = "marvell,armada-38x-uart", .data = (void *)DW_UART_QUIRK_ARMADA_38X }, { .compatible = "renesas,rzn1-uart" }, - { .compatible = "starfive,jh7100-uart" }, + { .compatible = "starfive,jh7100-uart", .data = (void *)DW_UART_QUIRK_SKIP_SET_RATE }, { /* Sentinel */ } }; MODULE_DEVICE_TABLE(of, dw8250_of_match);