On Mon, 21 Nov 2022 at 10:24, Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> wrote: > > On Sat, 19 Nov 2022 00:39:35 +0800, Emil Renner Berthing wrote: > > On Fri, 18 Nov 2022 at 02:06, Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> wrote: > > > diff --git a/drivers/reset/starfive/reset-starfive-jh7100.c b/drivers/reset/starfive/reset-starfive-jh71x0.c > > > similarity index 50% > > > copy from drivers/reset/starfive/reset-starfive-jh7100.c > > > copy to drivers/reset/starfive/reset-starfive-jh71x0.c > > > index fc44b2fb3e03..1e230f3f9841 100644 > > > --- a/drivers/reset/starfive/reset-starfive-jh7100.c > > > +++ b/drivers/reset/starfive/reset-starfive-jh71x0.c > > [...] > > > > -static int __init jh7100_reset_probe(struct platform_device *pdev) > > > +int reset_starfive_jh7100_register(struct device *dev, struct device_node *of_node, > > > + void __iomem *assert, void __iomem *status, > > > + const u64 *asserted, unsigned int nr_resets, > > > + bool is_module) > > > { > > > struct jh7100_reset *data; > > > > > > - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > > if (!data) > > > return -ENOMEM; > > > > > > - data->base = devm_platform_ioremap_resource(pdev, 0); > > > - if (IS_ERR(data->base)) > > > - return PTR_ERR(data->base); > > > - > > > data->rcdev.ops = &jh7100_reset_ops; > > > - data->rcdev.owner = THIS_MODULE; > > > - data->rcdev.nr_resets = JH7100_RSTN_END; > > > - data->rcdev.dev = &pdev->dev; > > > - data->rcdev.of_node = pdev->dev.of_node; > > > + if (is_module) > > > + data->rcdev.owner = THIS_MODULE; > > > > nit: consider just passing the owner directly, so this would just be > > data->rcdev.owner = owner; > > > > ..and callers that used false can just pass NULL. > > Yeah, will fix it. > > > > > > + data->rcdev.nr_resets = nr_resets; > > > + data->rcdev.dev = dev; > > > + data->rcdev.of_node = of_node; > > > > Is it important to register this with the auxiliary device and not > > just use the parent device? > > I'm not sure whether it still works if we use the same device, Try it. > but > it's general to separate the devices of clock and reset. They have > different device names and different drivers. > > Best regards, > Hal > > > If not you can just always pass the device that has the right of_node > > and have this be > > > > data->rcdev.of_node = dev->of_node; > > > > > + > > > spin_lock_init(&data->lock); > > > + data->assert = assert; > > > + data->status = status; > > > + data->asserted = asserted; > > > > > > - return devm_reset_controller_register(&pdev->dev, &data->rcdev); > > > + return devm_reset_controller_register(dev, &data->rcdev); > > > } >