| Message ID | 20210218200637.1677869-1-pbrobinson@gmail.com |
|---|---|
| State | Accepted |
| Commit | 391a16b710dd5688bffac1dca0026f96638361b0 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | rng: iProc rng200: Rename ..._platdata variables to just ..._plat | expand |
On 18.02.21 21:06, Peter Robinson wrote: > In 8a8d24bd Simon dropped data from all the various _platdata calls > but it seems this wasn't caught for the RNG200 driver from when it > was posted to merged. This fixes that issue. > > Fixes: 537f0018 (rng: Add iProc RNG200 driver) > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > --- > drivers/rng/iproc_rng200.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/rng/iproc_rng200.c b/drivers/rng/iproc_rng200.c > index f71f285f53..1126bbd797 100644 > --- a/drivers/rng/iproc_rng200.c > +++ b/drivers/rng/iproc_rng200.c > @@ -33,11 +33,11 @@ > #define RNG_FIFO_COUNT_OFFSET 0x24 > #define RNG_FIFO_COUNT_RNG_FIFO_COUNT_MASK 0x000000FF > > -struct iproc_rng200_platdata { > +struct iproc_rng200_plat { The renaming of the structure seems to be unrelated to the commit message. You are moving from allocating via plat_auto to priv_auto. So a reasonable name for the structure would be iproc_rng200_priv. > fdt_addr_t base; > }; > > -static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable) > +static void iproc_rng200_enable(struct iproc_rng200_plat *pdata, bool enable) > { > fdt_addr_t rng_base = pdata->base; > u32 val; > @@ -52,7 +52,7 @@ static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable > writel(val, rng_base + RNG_CTRL_OFFSET); > } > > -static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) > +static void iproc_rng200_restart(struct iproc_rng200_plat *pdata) > { > fdt_addr_t rng_base = pdata->base; > u32 val; > @@ -84,7 +84,7 @@ static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) > > static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) > { > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > + struct iproc_rng200_plat *priv = dev_get_plat(dev); At the end of the patch you move from plat_auto to priv_auto. So shouldn't you call dev_get_priv(dev)? > char *buf = (char *)data; > u32 num_remaining = len; > u32 status; > @@ -136,7 +136,7 @@ static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) > > static int iproc_rng200_probe(struct udevice *dev) > { > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > iproc_rng200_enable(priv, true); > > @@ -145,16 +145,16 @@ static int iproc_rng200_probe(struct udevice *dev) > > static int iproc_rng200_remove(struct udevice *dev) > { > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > iproc_rng200_enable(priv, false); > > return 0; > } > > -static int iproc_rng200_ofdata_to_platdata(struct udevice *dev) > +static int iproc_rng200_of_to_plat(struct udevice *dev) > { > - struct iproc_rng200_platdata *pdata = dev_get_plat(dev); > + struct iproc_rng200_plat *pdata = dev_get_plat(dev); > > pdata->base = dev_read_addr(dev); > if (!pdata->base) > @@ -180,6 +180,6 @@ U_BOOT_DRIVER(iproc_rng200_rng) = { > .ops = &iproc_rng200_ops, > .probe = iproc_rng200_probe, > .remove = iproc_rng200_remove, > - .plat_auto = sizeof(struct iproc_rng200_platdata), > - .of_to_plat = iproc_rng200_ofdata_to_platdata, > + .priv_auto = sizeof(struct iproc_rng200_plat), > + .of_to_plat = iproc_rng200_of_to_plat, Why do you set of_to_plat if you are using priv_auto data? I would have expected the probe function to fill the auto allocated private data. Best regards Heinrich > }; >
On Thu, Feb 18, 2021 at 8:36 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 18.02.21 21:06, Peter Robinson wrote: > > In 8a8d24bd Simon dropped data from all the various _platdata calls > > but it seems this wasn't caught for the RNG200 driver from when it > > was posted to merged. This fixes that issue. > > > > Fixes: 537f0018 (rng: Add iProc RNG200 driver) > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > > --- > > drivers/rng/iproc_rng200.c | 20 ++++++++++---------- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/rng/iproc_rng200.c b/drivers/rng/iproc_rng200.c > > index f71f285f53..1126bbd797 100644 > > --- a/drivers/rng/iproc_rng200.c > > +++ b/drivers/rng/iproc_rng200.c > > @@ -33,11 +33,11 @@ > > #define RNG_FIFO_COUNT_OFFSET 0x24 > > #define RNG_FIFO_COUNT_RNG_FIFO_COUNT_MASK 0x000000FF > > > > -struct iproc_rng200_platdata { > > +struct iproc_rng200_plat { > > The renaming of the structure seems to be unrelated to the commit message. > > You are moving from allocating via plat_auto to priv_auto. So a > reasonable name for the structure would be iproc_rng200_priv. > > > fdt_addr_t base; > > }; > > > > -static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable) > > +static void iproc_rng200_enable(struct iproc_rng200_plat *pdata, bool enable) > > { > > fdt_addr_t rng_base = pdata->base; > > u32 val; > > @@ -52,7 +52,7 @@ static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable > > writel(val, rng_base + RNG_CTRL_OFFSET); > > } > > > > -static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) > > +static void iproc_rng200_restart(struct iproc_rng200_plat *pdata) > > { > > fdt_addr_t rng_base = pdata->base; > > u32 val; > > @@ -84,7 +84,7 @@ static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) > > > > static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) > > { > > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > At the end of the patch you move from plat_auto to priv_auto. So > shouldn't you call dev_get_priv(dev)? > > > char *buf = (char *)data; > > u32 num_remaining = len; > > u32 status; > > @@ -136,7 +136,7 @@ static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) > > > > static int iproc_rng200_probe(struct udevice *dev) > > { > > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > > > iproc_rng200_enable(priv, true); > > > > @@ -145,16 +145,16 @@ static int iproc_rng200_probe(struct udevice *dev) > > > > static int iproc_rng200_remove(struct udevice *dev) > > { > > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > > > iproc_rng200_enable(priv, false); > > > > return 0; > > } > > > > -static int iproc_rng200_ofdata_to_platdata(struct udevice *dev) > > +static int iproc_rng200_of_to_plat(struct udevice *dev) > > { > > - struct iproc_rng200_platdata *pdata = dev_get_plat(dev); > > + struct iproc_rng200_plat *pdata = dev_get_plat(dev); > > > > pdata->base = dev_read_addr(dev); > > if (!pdata->base) > > @@ -180,6 +180,6 @@ U_BOOT_DRIVER(iproc_rng200_rng) = { > > .ops = &iproc_rng200_ops, > > .probe = iproc_rng200_probe, > > .remove = iproc_rng200_remove, > > - .plat_auto = sizeof(struct iproc_rng200_platdata), > > - .of_to_plat = iproc_rng200_ofdata_to_platdata, > > + .priv_auto = sizeof(struct iproc_rng200_plat), > > + .of_to_plat = iproc_rng200_of_to_plat, > > Why do you set of_to_plat if you are using priv_auto data? > I would have expected the probe function to fill the auto allocated > private data. I based the change on Simon's changes
On Thu, Feb 18, 2021 at 08:39:42PM +0000, Peter Robinson wrote: > On Thu, Feb 18, 2021 at 8:36 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > On 18.02.21 21:06, Peter Robinson wrote: > > > In 8a8d24bd Simon dropped data from all the various _platdata calls > > > but it seems this wasn't caught for the RNG200 driver from when it > > > was posted to merged. This fixes that issue. > > > > > > Fixes: 537f0018 (rng: Add iProc RNG200 driver) > > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > > > --- > > > drivers/rng/iproc_rng200.c | 20 ++++++++++---------- > > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/rng/iproc_rng200.c b/drivers/rng/iproc_rng200.c > > > index f71f285f53..1126bbd797 100644 > > > --- a/drivers/rng/iproc_rng200.c > > > +++ b/drivers/rng/iproc_rng200.c > > > @@ -33,11 +33,11 @@ > > > #define RNG_FIFO_COUNT_OFFSET 0x24 > > > #define RNG_FIFO_COUNT_RNG_FIFO_COUNT_MASK 0x000000FF > > > > > > -struct iproc_rng200_platdata { > > > +struct iproc_rng200_plat { > > > > The renaming of the structure seems to be unrelated to the commit message. > > > > You are moving from allocating via plat_auto to priv_auto. So a > > reasonable name for the structure would be iproc_rng200_priv. > > > > > fdt_addr_t base; > > > }; > > > > > > -static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable) > > > +static void iproc_rng200_enable(struct iproc_rng200_plat *pdata, bool enable) > > > { > > > fdt_addr_t rng_base = pdata->base; > > > u32 val; > > > @@ -52,7 +52,7 @@ static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable > > > writel(val, rng_base + RNG_CTRL_OFFSET); > > > } > > > > > > -static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) > > > +static void iproc_rng200_restart(struct iproc_rng200_plat *pdata) > > > { > > > fdt_addr_t rng_base = pdata->base; > > > u32 val; > > > @@ -84,7 +84,7 @@ static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) > > > > > > static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) > > > { > > > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > > > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > > > At the end of the patch you move from plat_auto to priv_auto. So > > shouldn't you call dev_get_priv(dev)? > > > > > char *buf = (char *)data; > > > u32 num_remaining = len; > > > u32 status; > > > @@ -136,7 +136,7 @@ static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) > > > > > > static int iproc_rng200_probe(struct udevice *dev) > > > { > > > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > > > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > > > > > iproc_rng200_enable(priv, true); > > > > > > @@ -145,16 +145,16 @@ static int iproc_rng200_probe(struct udevice *dev) > > > > > > static int iproc_rng200_remove(struct udevice *dev) > > > { > > > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > > > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > > > > > iproc_rng200_enable(priv, false); > > > > > > return 0; > > > } > > > > > > -static int iproc_rng200_ofdata_to_platdata(struct udevice *dev) > > > +static int iproc_rng200_of_to_plat(struct udevice *dev) > > > { > > > - struct iproc_rng200_platdata *pdata = dev_get_plat(dev); > > > + struct iproc_rng200_plat *pdata = dev_get_plat(dev); > > > > > > pdata->base = dev_read_addr(dev); > > > if (!pdata->base) > > > @@ -180,6 +180,6 @@ U_BOOT_DRIVER(iproc_rng200_rng) = { > > > .ops = &iproc_rng200_ops, > > > .probe = iproc_rng200_probe, > > > .remove = iproc_rng200_remove, > > > - .plat_auto = sizeof(struct iproc_rng200_platdata), > > > - .of_to_plat = iproc_rng200_ofdata_to_platdata, > > > + .priv_auto = sizeof(struct iproc_rng200_plat), > > > + .of_to_plat = iproc_rng200_of_to_plat, > > > > Why do you set of_to_plat if you are using priv_auto data? > > I would have expected the probe function to fill the auto allocated > > private data. > > I based the change on Simon's changes Yeah, this is fine, it was generally a global search and replace on all the structs and funcs. I'll pick this up (and some similar patches) soon, and just be a little annoyed at all the code that's clearly not getting built somehow today. It'd be great to figure out if it should be in some defconfig or what.
On Fri, Feb 19, 2021 at 04:00:50PM -0500, Tom Rini wrote: > On Thu, Feb 18, 2021 at 08:39:42PM +0000, Peter Robinson wrote: > > On Thu, Feb 18, 2021 at 8:36 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > On 18.02.21 21:06, Peter Robinson wrote: > > > > In 8a8d24bd Simon dropped data from all the various _platdata calls > > > > but it seems this wasn't caught for the RNG200 driver from when it > > > > was posted to merged. This fixes that issue. > > > > > > > > Fixes: 537f0018 (rng: Add iProc RNG200 driver) > > > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > > > > --- > > > > drivers/rng/iproc_rng200.c | 20 ++++++++++---------- > > > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/drivers/rng/iproc_rng200.c b/drivers/rng/iproc_rng200.c > > > > index f71f285f53..1126bbd797 100644 > > > > --- a/drivers/rng/iproc_rng200.c > > > > +++ b/drivers/rng/iproc_rng200.c > > > > @@ -33,11 +33,11 @@ > > > > #define RNG_FIFO_COUNT_OFFSET 0x24 > > > > #define RNG_FIFO_COUNT_RNG_FIFO_COUNT_MASK 0x000000FF > > > > > > > > -struct iproc_rng200_platdata { > > > > +struct iproc_rng200_plat { > > > > > > The renaming of the structure seems to be unrelated to the commit message. > > > > > > You are moving from allocating via plat_auto to priv_auto. So a > > > reasonable name for the structure would be iproc_rng200_priv. > > > > > > > fdt_addr_t base; > > > > }; > > > > > > > > -static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable) > > > > +static void iproc_rng200_enable(struct iproc_rng200_plat *pdata, bool enable) > > > > { > > > > fdt_addr_t rng_base = pdata->base; > > > > u32 val; > > > > @@ -52,7 +52,7 @@ static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable > > > > writel(val, rng_base + RNG_CTRL_OFFSET); > > > > } > > > > > > > > -static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) > > > > +static void iproc_rng200_restart(struct iproc_rng200_plat *pdata) > > > > { > > > > fdt_addr_t rng_base = pdata->base; > > > > u32 val; > > > > @@ -84,7 +84,7 @@ static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) > > > > > > > > static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) > > > > { > > > > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > > > > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > > > > > At the end of the patch you move from plat_auto to priv_auto. So > > > shouldn't you call dev_get_priv(dev)? > > > > > > > char *buf = (char *)data; > > > > u32 num_remaining = len; > > > > u32 status; > > > > @@ -136,7 +136,7 @@ static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) > > > > > > > > static int iproc_rng200_probe(struct udevice *dev) > > > > { > > > > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > > > > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > > > > > > > iproc_rng200_enable(priv, true); > > > > > > > > @@ -145,16 +145,16 @@ static int iproc_rng200_probe(struct udevice *dev) > > > > > > > > static int iproc_rng200_remove(struct udevice *dev) > > > > { > > > > - struct iproc_rng200_platdata *priv = dev_get_plat(dev); > > > > + struct iproc_rng200_plat *priv = dev_get_plat(dev); > > > > > > > > iproc_rng200_enable(priv, false); > > > > > > > > return 0; > > > > } > > > > > > > > -static int iproc_rng200_ofdata_to_platdata(struct udevice *dev) > > > > +static int iproc_rng200_of_to_plat(struct udevice *dev) > > > > { > > > > - struct iproc_rng200_platdata *pdata = dev_get_plat(dev); > > > > + struct iproc_rng200_plat *pdata = dev_get_plat(dev); > > > > > > > > pdata->base = dev_read_addr(dev); > > > > if (!pdata->base) > > > > @@ -180,6 +180,6 @@ U_BOOT_DRIVER(iproc_rng200_rng) = { > > > > .ops = &iproc_rng200_ops, > > > > .probe = iproc_rng200_probe, > > > > .remove = iproc_rng200_remove, > > > > - .plat_auto = sizeof(struct iproc_rng200_platdata), > > > > - .of_to_plat = iproc_rng200_ofdata_to_platdata, > > > > + .priv_auto = sizeof(struct iproc_rng200_plat), > > > > + .of_to_plat = iproc_rng200_of_to_plat, > > > > > > Why do you set of_to_plat if you are using priv_auto data? > > > I would have expected the probe function to fill the auto allocated > > > private data. > > > > I based the change on Simon's changes > > Yeah, this is fine, it was generally a global search and replace on all > the structs and funcs. I'll pick this up (and some similar patches) > soon, and just be a little annoyed at all the code that's clearly not > getting built somehow today. It'd be great to figure out if it should > be in some defconfig or what. > Tom, please keep me in the loop when you notify that you pushed this to master. I have some patches in my queue that will enable the iProc RNG200 driver for armv7 and plan to send it, after this got merged. Regards, Matthias
On Thu, Feb 18, 2021 at 08:06:37PM +0000, Peter Robinson wrote: > In 8a8d24bd Simon dropped data from all the various _platdata calls > but it seems this wasn't caught for the RNG200 driver from when it > was posted to merged. This fixes that issue. > > Fixes: 537f0018 (rng: Add iProc RNG200 driver) > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> Applied to u-boot/master, thanks!
diff --git a/drivers/rng/iproc_rng200.c b/drivers/rng/iproc_rng200.c index f71f285f53..1126bbd797 100644 --- a/drivers/rng/iproc_rng200.c +++ b/drivers/rng/iproc_rng200.c @@ -33,11 +33,11 @@ #define RNG_FIFO_COUNT_OFFSET 0x24 #define RNG_FIFO_COUNT_RNG_FIFO_COUNT_MASK 0x000000FF -struct iproc_rng200_platdata { +struct iproc_rng200_plat { fdt_addr_t base; }; -static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable) +static void iproc_rng200_enable(struct iproc_rng200_plat *pdata, bool enable) { fdt_addr_t rng_base = pdata->base; u32 val; @@ -52,7 +52,7 @@ static void iproc_rng200_enable(struct iproc_rng200_platdata *pdata, bool enable writel(val, rng_base + RNG_CTRL_OFFSET); } -static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) +static void iproc_rng200_restart(struct iproc_rng200_plat *pdata) { fdt_addr_t rng_base = pdata->base; u32 val; @@ -84,7 +84,7 @@ static void iproc_rng200_restart(struct iproc_rng200_platdata *pdata) static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) { - struct iproc_rng200_platdata *priv = dev_get_plat(dev); + struct iproc_rng200_plat *priv = dev_get_plat(dev); char *buf = (char *)data; u32 num_remaining = len; u32 status; @@ -136,7 +136,7 @@ static int iproc_rng200_read(struct udevice *dev, void *data, size_t len) static int iproc_rng200_probe(struct udevice *dev) { - struct iproc_rng200_platdata *priv = dev_get_plat(dev); + struct iproc_rng200_plat *priv = dev_get_plat(dev); iproc_rng200_enable(priv, true); @@ -145,16 +145,16 @@ static int iproc_rng200_probe(struct udevice *dev) static int iproc_rng200_remove(struct udevice *dev) { - struct iproc_rng200_platdata *priv = dev_get_plat(dev); + struct iproc_rng200_plat *priv = dev_get_plat(dev); iproc_rng200_enable(priv, false); return 0; } -static int iproc_rng200_ofdata_to_platdata(struct udevice *dev) +static int iproc_rng200_of_to_plat(struct udevice *dev) { - struct iproc_rng200_platdata *pdata = dev_get_plat(dev); + struct iproc_rng200_plat *pdata = dev_get_plat(dev); pdata->base = dev_read_addr(dev); if (!pdata->base) @@ -180,6 +180,6 @@ U_BOOT_DRIVER(iproc_rng200_rng) = { .ops = &iproc_rng200_ops, .probe = iproc_rng200_probe, .remove = iproc_rng200_remove, - .plat_auto = sizeof(struct iproc_rng200_platdata), - .of_to_plat = iproc_rng200_ofdata_to_platdata, + .priv_auto = sizeof(struct iproc_rng200_plat), + .of_to_plat = iproc_rng200_of_to_plat, };
In 8a8d24bd Simon dropped data from all the various _platdata calls but it seems this wasn't caught for the RNG200 driver from when it was posted to merged. This fixes that issue. Fixes: 537f0018 (rng: Add iProc RNG200 driver) Signed-off-by: Peter Robinson <pbrobinson@gmail.com> --- drivers/rng/iproc_rng200.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)