Re: [PATCH] mtd: rawnand: denali_dt: make the core clock optional
On 1/9/20 4:36 PM, Masahiro Yamada wrote:
> On Thu, Jan 9, 2020 at 10:20 PM Marek Vasut 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
>>> ---
>>>
>>> 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, );
>>> if (ret)
>>> - return ret;
>>> + clk.dev = NULL;
>>>
>>> ret = clk_get_by_name(dev, "nand_x", _x);
>>> if (ret)
>>> @@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev)
>>> if (ret)
>>> clk_ecc.dev = NULL;
>>>
>>> - ret = clk_enable();
>>> - if (ret)
>>> - return ret;
>>> + if (clk.dev) {
>>> + ret = clk_enable();
>>> + if (ret)
>>> + return ret;
>>> + }
>>>
>>> if (clk_x.dev) {
>>> ret = clk_enable(_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.
Re: [PATCH] mtd: rawnand: denali_dt: make the core clock optional
On Thu, Jan 9, 2020 at 10:20 PM Marek Vasut 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
> > ---
> >
> > 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, );
> > if (ret)
> > - return ret;
> > + clk.dev = NULL;
> >
> > ret = clk_get_by_name(dev, "nand_x", _x);
> > if (ret)
> > @@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev)
> > if (ret)
> > clk_ecc.dev = NULL;
> >
> > - ret = clk_enable();
> > - if (ret)
> > - return ret;
> > + if (clk.dev) {
> > + ret = clk_enable();
> > + if (ret)
> > + return ret;
> > + }
> >
> > if (clk_x.dev) {
> > ret = clk_enable(_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.
:
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: 9400 bl 0
18: aa0003f3 mov x19, x0
1c: aa1403e0 mov x0, x20
20: 9400 bl 0
24: b4e0 cbz x0, 40
28: b941 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: 9001 adrp x1, 0
48: 9100e3a2 add x2, x29, #0x38
4c: 9121 add x1, x1, #0x0
50: f9401680 ldr x0, [x20, #40]
54: 9400 bl 0
58: 350003e0 cbnz w0, d4
5c: f9401fa0 ldr x0, [x29, #56]
60: 9001 adrp x1, 0
64: f9036260 str x0, [x19, #1728]
68: 9100e3a2 add x2, x29, #0x38
6c: 9121 add x1, x1, #0x0
70: f9401680 ldr x0, [x20, #40]
74: 9400 bl 0
78: 350002e0 cbnz w0, d4
7c: f9401fa0 ldr x0, [x29, #56]
80: 911ca275 add x21, x19, #0x728
84: f9036660 str x0, [x19, #1736]
88: 9000 adrp x0, 0
8c: 9100 add x0, x0, #0x0
90: 9400 bl 0
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: 9400 bl 0
b8: 34000160 cbz w0, e4
bc: 2a0003e1 mov w1, w0
c0: 9000 adrp x0, 0
c4: 9100 add x0, x0, #0x0
c8: 9400 bl 0
cc: aa1303e0 mov x0, x19
d0: 9400 bl 0
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: 9400 bl 0
ec: 17f8 b cc
--
Best Regards
Masahiro Yamada
Re: [PATCH] mtd: rawnand: denali_dt: make the core clock optional
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
> ---
>
> 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, );
> if (ret)
> - return ret;
> + clk.dev = NULL;
>
> ret = clk_get_by_name(dev, "nand_x", _x);
> if (ret)
> @@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev)
> if (ret)
> clk_ecc.dev = NULL;
>
> - ret = clk_enable();
> - if (ret)
> - return ret;
> + if (clk.dev) {
> + ret = clk_enable();
> + if (ret)
> + return ret;
> + }
>
> if (clk_x.dev) {
> ret = clk_enable(_x);
>
Why do we need any of the clock code if the clock aren't available ? It
just takes space for no reason.
[PATCH] mtd: rawnand: denali_dt: make the core clock optional
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
---
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, );
if (ret)
- return ret;
+ clk.dev = NULL;
ret = clk_get_by_name(dev, "nand_x", _x);
if (ret)
@@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev)
if (ret)
clk_ecc.dev = NULL;
- ret = clk_enable();
- if (ret)
- return ret;
+ if (clk.dev) {
+ ret = clk_enable();
+ if (ret)
+ return ret;
+ }
if (clk_x.dev) {
ret = clk_enable(_x);
--
2.17.1
[PATCH] mtd: rawnand: denali_dt: make the core clock optional
From: Masahiro Yamada Masahiro Yamada
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 Masahiro Yamada
Signed-off-by: Masahiro Yamada
---
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, );
if (ret)
- return ret;
+ clk.dev = NULL;
ret = clk_get_by_name(dev, "nand_x", _x);
if (ret)
@@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev)
if (ret)
clk_ecc.dev = NULL;
- ret = clk_enable();
- if (ret)
- return ret;
+ if (clk.dev) {
+ ret = clk_enable();
+ if (ret)
+ return ret;
+ }
if (clk_x.dev) {
ret = clk_enable(_x);
--
2.17.1
