All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Daniel Lezcano <daniel.lezcano@linaro.org>, tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Kukjin Kim <kgene@kernel.org>,
	"moderated list:ARM/SAMSUNG EXYNO..."
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/SAMSUNG EXYNO..."
	<linux-samsung-soc@vger.kernel.org>
Subject: Re: [PATCH 5/9] clocksource/drivers/exynos_mct: Convert init function to return error
Date: Mon, 06 Jun 2016 13:28:57 +0200	[thread overview]
Message-ID: <57555E79.80606@samsung.com> (raw)
In-Reply-To: <57555D4A.8010001@linaro.org>

On 06/06/2016 01:23 PM, Daniel Lezcano wrote:
> On 06/06/2016 12:51 PM, Krzysztof Kozlowski wrote:
>> On 06/01/2016 10:34 AM, Daniel Lezcano wrote:
>>> The init functions do not return any error. They behave as the
>>> following:
>>>
>>>   - panic, thus leading to a kernel crash while another timer may
>>> work and
>>>     make the system boot up correctly
>>>
>>>   or
>>>
>>>   - print an error and let the caller unaware if the state of the system
>>>
>>> Change that by converting the init functions to return an error
>>> conforming
>>> to the CLOCKSOURCE_OF_RET prototype.
>>>
>>> Proper error handling (rollback, errno value) will be changed later case
>>> by case, thus this change just return back an error or success in the
>>> init
>>> function.
>>
>> I don't see the benefits of this change alone. You are replacing one
>> non-return code to another always-return-success code. The effect is
>> exactly the same. It would make sense to change it to CLOCKSOURCE_OF_RET
>> along with proper error handling.
>>
> 
> Hi Krzysztof,
> 
> I can understand you expect an error handling but, as stated in the
> changelog, proper error handling will be done later. Currently, there
> are roughly 80 drivers to changes the init function to return a value in
> order change the CLOCKSOURCE_OF_DECLARE macro. That is a quite important
> number and changing also the internals will introduce bugs.
> 
> So the series (and there is a huge one coming right after this one),
> will focus only on changing the init function to return a value, so we
> can swap the clocksource table and rename CLOCKSOURCE_OF_DECLARE_RET
> without the '_RET'. I want this to be done before the next PR in order
> to prevent to have a double clksrc table.
> 
> If error handling is already taken into account and is trivial, I try to
> do the change, otherwise I let the function to return a success value.
> 
> Regarding the important number of drivers, if you have time to change
> the init function in the exynos_mct, that will be more than welcome :)

Ah, okay, you are planning to get rid of CLOCKSOURCE_OF_DECLARE. I
didn't receive the cover letter so the big picture remained hidden. Now
it looks good:

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

WARNING: multiple messages have this Message-ID (diff)
From: k.kozlowski@samsung.com (Krzysztof Kozlowski)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 5/9] clocksource/drivers/exynos_mct: Convert init function to return error
Date: Mon, 06 Jun 2016 13:28:57 +0200	[thread overview]
Message-ID: <57555E79.80606@samsung.com> (raw)
In-Reply-To: <57555D4A.8010001@linaro.org>

On 06/06/2016 01:23 PM, Daniel Lezcano wrote:
> On 06/06/2016 12:51 PM, Krzysztof Kozlowski wrote:
>> On 06/01/2016 10:34 AM, Daniel Lezcano wrote:
>>> The init functions do not return any error. They behave as the
>>> following:
>>>
>>>   - panic, thus leading to a kernel crash while another timer may
>>> work and
>>>     make the system boot up correctly
>>>
>>>   or
>>>
>>>   - print an error and let the caller unaware if the state of the system
>>>
>>> Change that by converting the init functions to return an error
>>> conforming
>>> to the CLOCKSOURCE_OF_RET prototype.
>>>
>>> Proper error handling (rollback, errno value) will be changed later case
>>> by case, thus this change just return back an error or success in the
>>> init
>>> function.
>>
>> I don't see the benefits of this change alone. You are replacing one
>> non-return code to another always-return-success code. The effect is
>> exactly the same. It would make sense to change it to CLOCKSOURCE_OF_RET
>> along with proper error handling.
>>
> 
> Hi Krzysztof,
> 
> I can understand you expect an error handling but, as stated in the
> changelog, proper error handling will be done later. Currently, there
> are roughly 80 drivers to changes the init function to return a value in
> order change the CLOCKSOURCE_OF_DECLARE macro. That is a quite important
> number and changing also the internals will introduce bugs.
> 
> So the series (and there is a huge one coming right after this one),
> will focus only on changing the init function to return a value, so we
> can swap the clocksource table and rename CLOCKSOURCE_OF_DECLARE_RET
> without the '_RET'. I want this to be done before the next PR in order
> to prevent to have a double clksrc table.
> 
> If error handling is already taken into account and is trivial, I try to
> do the change, otherwise I let the function to return a success value.
> 
> Regarding the important number of drivers, if you have time to change
> the init function in the exynos_mct, that will be more than welcome :)

Ah, okay, you are planning to get rid of CLOCKSOURCE_OF_DECLARE. I
didn't receive the cover letter so the big picture remained hidden. Now
it looks good:

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

  reply	other threads:[~2016-06-06 11:28 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-01  8:34 [PATCH 0/9] clocksource/drivers/clksrc-of: Improve error handling Daniel Lezcano
2016-06-01  8:34 ` [PATCH 1/9] of: Add a new macro to declare_of for one parameter function returning a value Daniel Lezcano
2016-06-01  8:34   ` Daniel Lezcano
     [not found]   ` <1464770093-12667-2-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-06-14  7:41     ` Daniel Lezcano
2016-06-14  7:41       ` Daniel Lezcano
2016-06-01  8:34 ` [PATCH 2/9] clocksource/drivers/clksrc-probe: Introduce init functions with return code Daniel Lezcano
2016-06-01  8:34   ` Daniel Lezcano
     [not found] ` <1464770093-12667-1-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-06-01  8:34   ` [PATCH 3/9] clocksource/drivers/rockchip_timer: Convert init function to return error Daniel Lezcano
2016-06-01  8:34     ` Daniel Lezcano
2016-06-01  8:34     ` Daniel Lezcano
2016-06-01  8:34 ` [PATCH 4/9] clocksource/drivers/mkt_timer: " Daniel Lezcano
2016-06-01  8:34   ` Daniel Lezcano
2016-06-01  8:34   ` Daniel Lezcano
2016-06-01  8:34 ` [PATCH 5/9] clocksource/drivers/exynos_mct: " Daniel Lezcano
2016-06-01  8:34   ` Daniel Lezcano
2016-06-01  8:34   ` Daniel Lezcano
2016-06-06 10:51   ` Krzysztof Kozlowski
2016-06-06 10:51     ` Krzysztof Kozlowski
2016-06-06 11:23     ` Daniel Lezcano
2016-06-06 11:23       ` Daniel Lezcano
2016-06-06 11:28       ` Krzysztof Kozlowski [this message]
2016-06-06 11:28         ` Krzysztof Kozlowski
2016-06-01  8:34 ` [PATCH 6/9] clocksource/drivers/asm9260: " Daniel Lezcano
2016-06-01  8:34 ` [PATCH 7/9] clocksource/drivers/cadence_ttc: " Daniel Lezcano
2016-06-01  8:34   ` Daniel Lezcano
2016-06-01 14:36   ` Sören Brinkmann
2016-06-01 14:36     ` Sören Brinkmann
2016-06-01 14:43     ` Daniel Lezcano
2016-06-01 14:43       ` Daniel Lezcano
2016-06-01  8:34 ` [PATCH 8/9] clocksource/drivers/st_lpc: " Daniel Lezcano
2016-06-01  8:34   ` Daniel Lezcano
2016-06-01  8:34 ` [PATCH 9/9] clocksource/drivers/dw_apb_timer: " Daniel Lezcano
2016-06-07  9:54 ` [PATCH 0/9] clocksource/drivers/clksrc-of: Improve error handling Geert Uytterhoeven
2016-06-07 11:35   ` Daniel Lezcano
2016-06-07 22:23   ` Daniel Lezcano
2016-06-08 14:10   ` Daniel Lezcano
2016-06-09  7:46     ` Geert Uytterhoeven
2016-06-09  7:49       ` Daniel Lezcano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57555E79.80606@samsung.com \
    --to=k.kozlowski@samsung.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=kgene@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.