On Wed, Mar 30, 2022 at 03:20:32PM +0200, Miquel Raynal wrote:
> Before adding more platform data information, let's turn the quirks
> information as being a member of a wider structure. More fields will be
> added later.
>
> At the same time, change the quirks type to unsigned int, as its size
> won't change between setups and we don't really need this variable to be
> more than a few bits wide anyway.
>
> Provide a stub to the compatibles without quirks to simplify handling.
> Keep two different empty structure for the base DW compatible and the
> Renesas one because the latter will soon be fulfilled anyway.
I'm almost fine with it,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Please, see below remarks.
> Cc: Emil Renner Berthing <kernel@xxxxxxxx>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> ---
> drivers/tty/serial/8250/8250_dw.c | 32 ++++++++++++++++++++++------
> drivers/tty/serial/8250/8250_dwlib.h | 5 +++++
> 2 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index b6238b9bf0b2..70a843e31ffd 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -19,6 +19,7 @@
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> +#include <linux/property.h>
> #include <linux/workqueue.h>
> #include <linux/notifier.h>
> #include <linux/slab.h>
> @@ -371,7 +372,7 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
> static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
> {
> struct device_node *np = p->dev->of_node;
> - unsigned long quirks = (unsigned long)device_get_match_data(p->dev);
> + unsigned int quirks = data->pdata->quirks;
>
> if (np) {
> int id;
> @@ -462,6 +463,7 @@ static int dw8250_probe(struct platform_device *pdev)
>
> data->data.dma.fn = dw8250_fallback_dma_filter;
> data->usr_reg = DW_UART_USR;
(1)
> + data->pdata = device_get_match_data(p->dev);
> p->private_data = &data->data;
>
> data->uart_16550_compatible = device_property_read_bool(dev,
> @@ -678,13 +680,29 @@ static const struct dev_pm_ops dw8250_pm_ops = {
> SET_RUNTIME_PM_OPS(dw8250_runtime_suspend, dw8250_runtime_resume, NULL)
> };
>
> +static const struct dw8250_platform_data dw8250_dw_apb = {};
(2)
' = {}' part is actually redundant in (2), but can we move (1) to pdata
(perhaps as a separate patch)?
In such case I would change (2) to
static const struct dw8250_platform_data dw8250_dw_apb = {
};
> +static const struct dw8250_platform_data dw8250_octeon_3860_data = {
> + .quirks = DW_UART_QUIRK_OCTEON,
> +};
> +
> +static const struct dw8250_platform_data dw8250_armada_38x_data = {
> + .quirks = DW_UART_QUIRK_ARMADA_38X,
> +};
> +
> +static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {};
> +
> +static const struct dw8250_platform_data dw8250_starfive_jh7100_data = {
> + .quirks = DW_UART_QUIRK_SKIP_SET_RATE,
> +};
> +
> static const struct of_device_id dw8250_of_match[] = {
> - { .compatible = "snps,dw-apb-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-hsuart", .data = (void *)DW_UART_QUIRK_SKIP_SET_RATE },
> - { .compatible = "starfive,jh7100-uart", .data = (void *)DW_UART_QUIRK_SKIP_SET_RATE },
> + { .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
> + { .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-hsuart", .data = &dw8250_starfive_jh7100_data },
> + { .compatible = "starfive,jh7100-uart", .data = &dw8250_starfive_jh7100_data },
> { /* Sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, dw8250_of_match);
> diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
> index 72e7dbcccad0..68bb81bee660 100644
> --- a/drivers/tty/serial/8250/8250_dwlib.h
> +++ b/drivers/tty/serial/8250/8250_dwlib.h
> @@ -21,8 +21,13 @@ struct dw8250_port_data {
> u8 dlf_size;
> };
>
> +struct dw8250_platform_data {
> + unsigned int quirks;
> +};
> +
> struct dw8250_data {
> struct dw8250_port_data data;
> + const struct dw8250_platform_data *pdata;
>
> u8 usr_reg;
> int msr_mask_on;
> --
> 2.27.0
>
--
With Best Regards,
Andy Shevchenko
