| Message ID | 20200109120206.18060-1-yamada.masahiro@socionext.com |
|---|---|
| State | Accepted |
| Commit | 11bcc5841ae6765e010a419bd6354b15ae4e1096 |
| Headers | show |
| Series | mtd: rawnand: denali_dt: make the core clock optional | expand |
On 1/9/20 1:02 PM, Masahiro Yamada wrote: > The "nand_x" and "ecc" clocks are currently optional. Make the core > clock optional in the same way. This will allow platforms with no clock > driver support to use this driver. > > Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com> > --- > > drivers/mtd/nand/raw/denali_dt.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c > index 0ce81324b90e..b1e14982c443 100644 > --- a/drivers/mtd/nand/raw/denali_dt.c > +++ b/drivers/mtd/nand/raw/denali_dt.c > @@ -91,7 +91,7 @@ static int denali_dt_probe(struct udevice *dev) > if (ret) > ret = clk_get_by_index(dev, 0, &clk); > if (ret) > - return ret; > + clk.dev = NULL; > > ret = clk_get_by_name(dev, "nand_x", &clk_x); > if (ret) > @@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev) > if (ret) > clk_ecc.dev = NULL; > > - ret = clk_enable(&clk); > - if (ret) > - return ret; > + if (clk.dev) { > + ret = clk_enable(&clk); > + if (ret) > + return ret; > + } > > if (clk_x.dev) { > ret = clk_enable(&clk_x); > Why do we need any of the clock code if the clock aren't available ? It just takes space for no reason.
On Thu, Jan 9, 2020 at 10:20 PM Marek Vasut <marex at denx.de> wrote: > > On 1/9/20 1:02 PM, Masahiro Yamada wrote: > > The "nand_x" and "ecc" clocks are currently optional. Make the core > > clock optional in the same way. This will allow platforms with no clock > > driver support to use this driver. > > > > Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com> > > --- > > > > drivers/mtd/nand/raw/denali_dt.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c > > index 0ce81324b90e..b1e14982c443 100644 > > --- a/drivers/mtd/nand/raw/denali_dt.c > > +++ b/drivers/mtd/nand/raw/denali_dt.c > > @@ -91,7 +91,7 @@ static int denali_dt_probe(struct udevice *dev) > > if (ret) > > ret = clk_get_by_index(dev, 0, &clk); > > if (ret) > > - return ret; > > + clk.dev = NULL; > > > > ret = clk_get_by_name(dev, "nand_x", &clk_x); > > if (ret) > > @@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev) > > if (ret) > > clk_ecc.dev = NULL; > > > > - ret = clk_enable(&clk); > > - if (ret) > > - return ret; > > + if (clk.dev) { > > + ret = clk_enable(&clk); > > + if (ret) > > + return ret; > > + } > > > > if (clk_x.dev) { > > ret = clk_enable(&clk_x); > > > > Why do we need any of the clock code if the clock aren't available ? It > just takes space for no reason. Are you sure? The compiler optimizes the unreachable code out. My compiler is this: $ aarch64-linux-gnu-gcc --version aarch64-linux-gnu-gcc (Linaro GCC 7.4-2019.02) 7.4.1 20181213 [linaro-7.4-2019.02 revision 56ec6f6b99cc167ff0c2f8e1a2eed33b1edc85d4] Copyright (C) 2017 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. I compiled the denali_dt.c with CONFIG_CLK disabled and here is the disassembly. My patch and yours produce exactly the same code. 0000000000000000 <denali_dt_probe>: 0: a9b97bfd stp x29, x30, [sp, #-112]! 4: 910003fd mov x29, sp 8: a90153f3 stp x19, x20, [sp, #16] c: aa0003f4 mov x20, x0 10: f90013f5 str x21, [sp, #32] 14: 94000000 bl 0 <dev_get_priv> 18: aa0003f3 mov x19, x0 1c: aa1403e0 mov x0, x20 20: 94000000 bl 0 <dev_get_driver_data> 24: b40000e0 cbz x0, 40 <denali_dt_probe+0x40> 28: b9400001 ldr w1, [x0] 2c: b9070261 str w1, [x19, #1792] 30: b9400401 ldr w1, [x0, #4] 34: b9070661 str w1, [x19, #1796] 38: f9400400 ldr x0, [x0, #8] 3c: f9038660 str x0, [x19, #1800] 40: f9035a74 str x20, [x19, #1712] 44: 90000001 adrp x1, 0 <denali_dt_probe> 48: 9100e3a2 add x2, x29, #0x38 4c: 91000021 add x1, x1, #0x0 50: f9401680 ldr x0, [x20, #40] 54: 94000000 bl 0 <ofnode_read_resource_byname> 58: 350003e0 cbnz w0, d4 <denali_dt_probe+0xd4> 5c: f9401fa0 ldr x0, [x29, #56] 60: 90000001 adrp x1, 0 <denali_dt_probe> 64: f9036260 str x0, [x19, #1728] 68: 9100e3a2 add x2, x29, #0x38 6c: 91000021 add x1, x1, #0x0 70: f9401680 ldr x0, [x20, #40] 74: 94000000 bl 0 <ofnode_read_resource_byname> 78: 350002e0 cbnz w0, d4 <denali_dt_probe+0xd4> 7c: f9401fa0 ldr x0, [x29, #56] 80: 911ca275 add x21, x19, #0x728 84: f9036660 str x0, [x19, #1736] 88: 90000000 adrp x0, 0 <denali_dt_probe> 8c: 91000000 add x0, x0, #0x0 90: 94000000 bl 0 <printf> 94: d29e1000 mov x0, #0xf080 // #61568 98: aa1503e1 mov x1, x21 9c: f2a05f40 movk x0, #0x2fa, lsl #16 a0: f9034e60 str x0, [x19, #1688] a4: d2984000 mov x0, #0xc200 // #49664 a8: f2a17d60 movk x0, #0xbeb, lsl #16 ac: f9035260 str x0, [x19, #1696] b0: aa1403e0 mov x0, x20 b4: 94000000 bl 0 <reset_get_bulk> b8: 34000160 cbz w0, e4 <denali_dt_probe+0xe4> bc: 2a0003e1 mov w1, w0 c0: 90000000 adrp x0, 0 <denali_dt_probe> c4: 91000000 add x0, x0, #0x0 c8: 94000000 bl 0 <printf> cc: aa1303e0 mov x0, x19 d0: 94000000 bl 0 <denali_init> d4: a94153f3 ldp x19, x20, [sp, #16] d8: f94013f5 ldr x21, [sp, #32] dc: a8c77bfd ldp x29, x30, [sp], #112 e0: d65f03c0 ret e4: aa1503e0 mov x0, x21 e8: 94000000 bl 0 <reset_deassert_bulk> ec: 17fffff8 b cc <denali_dt_probe+0xcc>
On 1/9/20 4:36 PM, Masahiro Yamada wrote: > On Thu, Jan 9, 2020 at 10:20 PM Marek Vasut <marex at denx.de> wrote: >> >> On 1/9/20 1:02 PM, Masahiro Yamada wrote: >>> The "nand_x" and "ecc" clocks are currently optional. Make the core >>> clock optional in the same way. This will allow platforms with no clock >>> driver support to use this driver. >>> >>> Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com> >>> --- >>> >>> drivers/mtd/nand/raw/denali_dt.c | 10 ++++++---- >>> 1 file changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c >>> index 0ce81324b90e..b1e14982c443 100644 >>> --- a/drivers/mtd/nand/raw/denali_dt.c >>> +++ b/drivers/mtd/nand/raw/denali_dt.c >>> @@ -91,7 +91,7 @@ static int denali_dt_probe(struct udevice *dev) >>> if (ret) >>> ret = clk_get_by_index(dev, 0, &clk); >>> if (ret) >>> - return ret; >>> + clk.dev = NULL; >>> >>> ret = clk_get_by_name(dev, "nand_x", &clk_x); >>> if (ret) >>> @@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev) >>> if (ret) >>> clk_ecc.dev = NULL; >>> >>> - ret = clk_enable(&clk); >>> - if (ret) >>> - return ret; >>> + if (clk.dev) { >>> + ret = clk_enable(&clk); >>> + if (ret) >>> + return ret; >>> + } >>> >>> if (clk_x.dev) { >>> ret = clk_enable(&clk_x); >>> >> >> Why do we need any of the clock code if the clock aren't available ? It >> just takes space for no reason. > > Are you sure? > > The compiler optimizes the unreachable code out. > > > My compiler is this: > > $ aarch64-linux-gnu-gcc --version > aarch64-linux-gnu-gcc (Linaro GCC 7.4-2019.02) 7.4.1 20181213 > [linaro-7.4-2019.02 revision 56ec6f6b99cc167ff0c2f8e1a2eed33b1edc85d4] > Copyright (C) 2017 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > > > > I compiled the denali_dt.c with CONFIG_CLK disabled > and here is the disassembly. > > My patch and yours produce exactly the same code. Ha, there's indeed no increase. I tested this patch and it works too. Thanks I sent this again as part of the reworked series with a T-B.
diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index 0ce81324b90e..b1e14982c443 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -91,7 +91,7 @@ static int denali_dt_probe(struct udevice *dev) if (ret) ret = clk_get_by_index(dev, 0, &clk); if (ret) - return ret; + clk.dev = NULL; ret = clk_get_by_name(dev, "nand_x", &clk_x); if (ret) @@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev) if (ret) clk_ecc.dev = NULL; - ret = clk_enable(&clk); - if (ret) - return ret; + if (clk.dev) { + ret = clk_enable(&clk); + if (ret) + return ret; + } if (clk_x.dev) { ret = clk_enable(&clk_x);
The "nand_x" and "ecc" clocks are currently optional. Make the core clock optional in the same way. This will allow platforms with no clock driver support to use this driver. Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com> --- drivers/mtd/nand/raw/denali_dt.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)