Hi Prabhakar, On Tue, Nov 9, 2021 at 1:17 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote: > On RZ/G2L SoC we need to explicitly deassert the reset line > for the device to work, use this opportunity to deassert/assert > reset line in sh-sci driver. > > This patch adds support to read the "resets" property (if available) > from DT and perform deassert/assert when required. > > Also, propagate the error to the caller of sci_parse_dt() instead of > returning NULL in case of failure. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > --- > v1->v2 > * deassert/assert reset line if available on all SoC's > * Updated commit message Thanks for the update! > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -37,6 +37,7 @@ > #include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > +#include <linux/reset.h> > #include <linux/scatterlist.h> > #include <linux/serial.h> > #include <linux/serial_sci.h> > @@ -3203,23 +3204,53 @@ static const struct of_device_id of_sci_match[] = { > }; > MODULE_DEVICE_TABLE(of, of_sci_match); > > +static void sci_reset_control_assert(void *data) > +{ > + reset_control_assert(data); > +} > + > static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev, > unsigned int *dev_id) > { > struct device_node *np = pdev->dev.of_node; > + const struct of_device_id *of_id; Not needed. > + struct reset_control *rstc; > struct plat_sci_port *p; > struct sci_port *sp; > const void *data; > - int id; > + int id, ret; > > if (!IS_ENABLED(CONFIG_OF) || !np) > - return NULL; > + return ERR_PTR(-EINVAL); > + > + of_id = of_match_device(of_sci_match, &pdev->dev); > + if (!of_id) > + return ERR_PTR(-EINVAL); > > - data = of_device_get_match_data(&pdev->dev); Please keep the old construct using of_device_get_match_data(). > + rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL); > + if (IS_ERR(rstc)) { > + dev_err(&pdev->dev, "failed to get reset ctrl %ld\n", PTR_ERR(rstc)); > + return ERR_PTR(PTR_ERR(rstc)); The error might be -EPROBE_DEFER, so please use "return ERR_PTR(dev_err_probe(...))", to avoid printing the message in case of probe deferral. BTW, ERR_CAST() is a shorthand for ERR_PTR(PTR_ERR()). > + } > + > + ret = reset_control_deassert(rstc); > + if (ret) { > + dev_err(&pdev->dev, "failed to deassert reset %d\n", ret); > + return ERR_PTR(ret); > + } > + > + ret = devm_add_action_or_reset(&pdev->dev, sci_reset_control_assert, rstc); > + if (ret) { > + dev_err(&pdev->dev, "failed to register assert devm action, %d\n", > + ret); > + return ERR_PTR(ret); > + } > + > + data = of_id->data; > > p = devm_kzalloc(&pdev->dev, sizeof(struct plat_sci_port), GFP_KERNEL); > if (!p) > - return NULL; > + return ERR_PTR(-ENOMEM); > > /* Get the line number from the aliases node. */ > id = of_alias_get_id(np, "serial"); I gave this a try on Salvator-XS, and noticed no regressions. I will test this on more SCIF variants later. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds