On 15 August 2013 14:42, Markos Chandras <markos.chandras@xxxxxxxxx> wrote:
> On 14 August 2013 12:12, Jonas Gorski <jogo@xxxxxxxxxxx> wrote:
>> Hi,
>>
>> On Tue, Aug 13, 2013 at 11:01 AM, Markos Chandras
>> <markos.chandras@xxxxxxxxxx> wrote:
>>> Fixes the following build error:
>>> arch/mips/include/asm/mach-ath79/ath79.h:139:20: error: 'reg' may be used
>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>> arch/mips/ath79/common.c:62:6: note: 'reg' was declared here
>>> In file included from arch/mips/ath79/common.c:20:0:
>>> arch/mips/ath79/common.c: In function 'ath79_device_reset_clear':
>>> arch/mips/include/asm/mach-ath79/ath79.h:139:20:
>>> error: 'reg' may be used uninitialized in this function
>>> [-Werror=maybe-uninitialized]
>>> arch/mips/ath79/common.c:90:6: note: 'reg' was declared here
>>>
>>> Signed-off-by: Markos Chandras <markos.chandras@xxxxxxxxxx>
>>> ---
>>> This patch is for the upstream-sfr/mips-for-linux-next tree
>>> ---
>>> arch/mips/ath79/common.c | 32 ++++++++++++++++++--------------
>>> 1 file changed, 18 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/arch/mips/ath79/common.c b/arch/mips/ath79/common.c
>>> index eb3966c..6a8c00f 100644
>>> --- a/arch/mips/ath79/common.c
>>> +++ b/arch/mips/ath79/common.c
>>> @@ -62,20 +62,22 @@ void ath79_device_reset_set(u32 mask)
>>> u32 reg;
>>> u32 t;
>>>
>>> - if (soc_is_ar71xx())
>>> + if (soc_is_ar71xx()) {
>>> reg = AR71XX_RESET_REG_RESET_MODULE;
>>> - else if (soc_is_ar724x())
>>> + } else if (soc_is_ar724x()) {
>>> reg = AR724X_RESET_REG_RESET_MODULE;
>>> - else if (soc_is_ar913x())
>>> + } else if (soc_is_ar913x()) {
>>> reg = AR913X_RESET_REG_RESET_MODULE;
>>> - else if (soc_is_ar933x())
>>> + } else if (soc_is_ar933x()) {
>>> reg = AR933X_RESET_REG_RESET_MODULE;
>>> - else if (soc_is_ar934x())
>>> + } else if (soc_is_ar934x()) {
>>> reg = AR934X_RESET_REG_RESET_MODULE;
>>> - else if (soc_is_qca955x())
>>> + } else if (soc_is_qca955x()) {
>>> reg = QCA955X_RESET_REG_RESET_MODULE;
>>> - else
>>> + } else {
>>> BUG();
>>> + panic("Unknown SOC!");
>>
>> Both BUG() and panic() seems to be a bit overkill, especially since
>> the panic won't be reached unless BUG is disabled - just the panic()
>> should be enough.
>>
>> Also the panic message isn't very helpful, maybe print the raw id of
>> the SoC here?
>>
>>
>
> Hi Jonas,
>
> Thank you for the review. I will submit a new patch.
>
> --
> Regards,
> Markos Chandras
Hi Jonas,
I had a look at the code again and it seems that reporting the 'id' is
not needed since an unknown SOC will cause a panic
earlier in the boot process. Look at arch/mips/ath79/setup.c, in the
plat_mem_setup function.
This one calls ath79_detect_sys_type which causes the following panic
if an unknown SOC is detected.
panic("ath79: unknown SoC, id:0x%08x", id);
This makes me think that ath79_device_reset_set and
ath79_device_reset_clear should not care about an unknown SOC at all.
--
Regards,
Markos Chandras
