mbox series

[0/7] clk: at91: sckc: improve error path

Message ID 1560440205-4604-1-git-send-email-claudiu.beznea@microchip.com (mailing list archive)
Headers show
Series clk: at91: sckc: improve error path | expand

Message

Claudiu Beznea June 13, 2019, 3:37 p.m. UTC
From: Claudiu Beznea <claudiu.beznea@microchip.com>

Hi,

This series tries to improve error path for slow clock registrations
by adding functions to free resources and using them on failures.

It is created on top of patch series at [1].

Thank you,
Claudiu Beznea

[1] https://lore.kernel.org/lkml/1558433454-27971-1-git-send-email-claudiu.beznea@microchip.com/

Claudiu Beznea (7):
  clk: at91: sckc: add support to free slow oscillator
  clk: at91: sckc: add support to free slow rc oscillator
  clk: at91: sckc: add support to free slow clock osclillator
  clk: at91: sckc: improve error path for sam9x5 sck register
  clk: at91: sckc: remove unnecessary line
  clk: at91: sckc: improve error path for sama5d4 sck registration
  clk: at91: sckc: use dedicated functions to unregister clock

 drivers/clk/at91/sckc.c | 122 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 86 insertions(+), 36 deletions(-)

Comments

Alexandre Belloni June 18, 2019, 9:55 a.m. UTC | #1
On 13/06/2019 15:37:06+0000, Claudiu.Beznea@microchip.com wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Hi,
> 
> This series tries to improve error path for slow clock registrations
> by adding functions to free resources and using them on failures.
> 

Does the platform even boot when the slow clock is not available? 

The TCB clocksource would fail at:

        tc.slow_clk = of_clk_get_by_name(node->parent, "slow_clk");
        if (IS_ERR(tc.slow_clk))
                return PTR_ERR(tc.slow_clk);


> It is created on top of patch series at [1].
> 
> Thank you,
> Claudiu Beznea
> 
> [1] https://lore.kernel.org/lkml/1558433454-27971-1-git-send-email-claudiu.beznea@microchip.com/
> 
> Claudiu Beznea (7):
>   clk: at91: sckc: add support to free slow oscillator
>   clk: at91: sckc: add support to free slow rc oscillator
>   clk: at91: sckc: add support to free slow clock osclillator
>   clk: at91: sckc: improve error path for sam9x5 sck register
>   clk: at91: sckc: remove unnecessary line
>   clk: at91: sckc: improve error path for sama5d4 sck registration
>   clk: at91: sckc: use dedicated functions to unregister clock
> 
>  drivers/clk/at91/sckc.c | 122 ++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 86 insertions(+), 36 deletions(-)
> 
> -- 
> 2.7.4
>
Claudiu Beznea June 20, 2019, 10:30 a.m. UTC | #2
Hi,

On 18.06.2019 12:55, Alexandre Belloni wrote:
> On 13/06/2019 15:37:06+0000, Claudiu.Beznea@microchip.com wrote:
>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
>> Hi,
>>
>> This series tries to improve error path for slow clock registrations
>> by adding functions to free resources and using them on failures.
>>
> 
> Does the platform even boot when the slow clock is not available? 
> 
> The TCB clocksource would fail at:
> 
>         tc.slow_clk = of_clk_get_by_name(node->parent, "slow_clk");
>         if (IS_ERR(tc.slow_clk))
>                 return PTR_ERR(tc.slow_clk);
> 

In case of using TC as clocksource, yes, the platform wouldn't boot if slow
clock is not available, because, anyway the TC needs it. PIT may work
without it (if slow clock is not used to drive the PIT).

For sure there are other IPs (which may be or are driven by slow clock)
which may not work if slow clock is driven them.

Anyway, please let me know if you feel this series has no meaning.

Thank you,
Claudiu Beznea

> 
>> It is created on top of patch series at [1].
>>
>> Thank you,
>> Claudiu Beznea
>>
>> [1] https://lore.kernel.org/lkml/1558433454-27971-1-git-send-email-claudiu.beznea@microchip.com/
>>
>> Claudiu Beznea (7):
>>   clk: at91: sckc: add support to free slow oscillator
>>   clk: at91: sckc: add support to free slow rc oscillator
>>   clk: at91: sckc: add support to free slow clock osclillator
>>   clk: at91: sckc: improve error path for sam9x5 sck register
>>   clk: at91: sckc: remove unnecessary line
>>   clk: at91: sckc: improve error path for sama5d4 sck registration
>>   clk: at91: sckc: use dedicated functions to unregister clock
>>
>>  drivers/clk/at91/sckc.c | 122 ++++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 86 insertions(+), 36 deletions(-)
>>
>> -- 
>> 2.7.4
>>
>
Alexandre Belloni June 21, 2019, 9:33 a.m. UTC | #3
On 20/06/2019 10:30:42+0000, Claudiu.Beznea@microchip.com wrote:
> Hi,
> 
> On 18.06.2019 12:55, Alexandre Belloni wrote:
> > On 13/06/2019 15:37:06+0000, Claudiu.Beznea@microchip.com wrote:
> >> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> >>
> >> Hi,
> >>
> >> This series tries to improve error path for slow clock registrations
> >> by adding functions to free resources and using them on failures.
> >>
> > 
> > Does the platform even boot when the slow clock is not available? 
> > 
> > The TCB clocksource would fail at:
> > 
> >         tc.slow_clk = of_clk_get_by_name(node->parent, "slow_clk");
> >         if (IS_ERR(tc.slow_clk))
> >                 return PTR_ERR(tc.slow_clk);
> > 
> 
> In case of using TC as clocksource, yes, the platform wouldn't boot if slow
> clock is not available, because, anyway the TC needs it. PIT may work
> without it (if slow clock is not used to drive the PIT).
> 
> For sure there are other IPs (which may be or are driven by slow clock)
> which may not work if slow clock is driven them.
> 
> Anyway, please let me know if you feel this series has no meaning.
> 

Well, I'm not sure it is worth it but at the same time, it is not adding
many lines and you already developed it...
Stephen Boyd June 26, 2019, 6:53 p.m. UTC | #4
Quoting Alexandre Belloni (2019-06-21 02:33:02)
> On 20/06/2019 10:30:42+0000, Claudiu.Beznea@microchip.com wrote:
> > Hi,
> > 
> > On 18.06.2019 12:55, Alexandre Belloni wrote:
> > > On 13/06/2019 15:37:06+0000, Claudiu.Beznea@microchip.com wrote:
> > >> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> > >>
> > >> Hi,
> > >>
> > >> This series tries to improve error path for slow clock registrations
> > >> by adding functions to free resources and using them on failures.
> > >>
> > > 
> > > Does the platform even boot when the slow clock is not available? 
> > > 
> > > The TCB clocksource would fail at:
> > > 
> > >         tc.slow_clk = of_clk_get_by_name(node->parent, "slow_clk");
> > >         if (IS_ERR(tc.slow_clk))
> > >                 return PTR_ERR(tc.slow_clk);
> > > 
> > 
> > In case of using TC as clocksource, yes, the platform wouldn't boot if slow
> > clock is not available, because, anyway the TC needs it. PIT may work
> > without it (if slow clock is not used to drive the PIT).
> > 
> > For sure there are other IPs (which may be or are driven by slow clock)
> > which may not work if slow clock is driven them.
> > 
> > Anyway, please let me know if you feel this series has no meaning.
> > 
> 
> Well, I'm not sure it is worth it but at the same time, it is not adding
> many lines and you already developed it...
> 

Is that a Reviewed-by or a Rejected-by tag?
Alexandre Belloni June 26, 2019, 7 p.m. UTC | #5
On 26/06/2019 11:53:59-0700, Stephen Boyd wrote:
> Quoting Alexandre Belloni (2019-06-21 02:33:02)
> > On 20/06/2019 10:30:42+0000, Claudiu.Beznea@microchip.com wrote:
> > > Hi,
> > > 
> > > On 18.06.2019 12:55, Alexandre Belloni wrote:
> > > > On 13/06/2019 15:37:06+0000, Claudiu.Beznea@microchip.com wrote:
> > > >> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> > > >>
> > > >> Hi,
> > > >>
> > > >> This series tries to improve error path for slow clock registrations
> > > >> by adding functions to free resources and using them on failures.
> > > >>
> > > > 
> > > > Does the platform even boot when the slow clock is not available? 
> > > > 
> > > > The TCB clocksource would fail at:
> > > > 
> > > >         tc.slow_clk = of_clk_get_by_name(node->parent, "slow_clk");
> > > >         if (IS_ERR(tc.slow_clk))
> > > >                 return PTR_ERR(tc.slow_clk);
> > > > 
> > > 
> > > In case of using TC as clocksource, yes, the platform wouldn't boot if slow
> > > clock is not available, because, anyway the TC needs it. PIT may work
> > > without it (if slow clock is not used to drive the PIT).
> > > 
> > > For sure there are other IPs (which may be or are driven by slow clock)
> > > which may not work if slow clock is driven them.
> > > 
> > > Anyway, please let me know if you feel this series has no meaning.
> > > 
> > 
> > Well, I'm not sure it is worth it but at the same time, it is not adding
> > many lines and you already developed it...
> > 
> 
> Is that a Reviewed-by or a Rejected-by tag?
> 

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Stephen Boyd June 27, 2019, 3:03 p.m. UTC | #6
Quoting Claudiu.Beznea@microchip.com (2019-06-13 08:37:06)
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Hi,
> 
> This series tries to improve error path for slow clock registrations
> by adding functions to free resources and using them on failures.

If possible, resend this patch series in plain text. Thanks.
Claudiu Beznea June 27, 2019, 3:55 p.m. UTC | #7
On 27.06.2019 18:03, Stephen Boyd wrote:
> External E-Mail
> 
> 
> Quoting Claudiu.Beznea@microchip.com (2019-06-13 08:37:06)
>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
>> Hi,
>>
>> This series tries to improve error path for slow clock registrations
>> by adding functions to free resources and using them on failures.
> 
> If possible, resend this patch series in plain text. Thanks.

Done! Thank you!

>