[PATCH v3 5/6] mmc: actions: add MMC driver for Actions OWL S700/S900

[Jaehoon Chung]

On 3/2/21 8:04 AM, Andre Przywara wrote:
> On Tue, 2 Mar 2021 07:11:54 +0900
> Jaehoon Chung <jh80.chung at samsung.com> wrote:
> 
>> On 3/1/21 11:25 PM, Andre Przywara wrote:
>>> On Mon, 1 Mar 2021 18:47:26 +0530
>>> Amit Tomar <atomar25opensource at gmail.com> wrote:
>>>
>>> Hi,
>>>   
>>>>> So this whole mode handling here looks dodgy. Below you mix "assignments
>>>>> to mode" with "ORing in values", without actually ever initialising mode
>>>>> explicitly. I wonder why the compiler doesn't warn about this, I can see
>>>>> paths were you OR into an uninitialised value.
>>>>>
>>>>> But the compiler already has initialized mode to 0,  
>>>
>>> Why? Where? I just see a local, non-static definition of mode,
>>> meaning it won't be initialised.  
>>
>> It seems that it doesn't initialize in this function.
>> I think that resp_type may be always matched one of condition.
> 
> Possibly, but the problem is that the MMC_RSP_R1 clause also uses
> "|=", so it's the same issue here.
> One a first glance into the assembly it looks like the compiler
> optimises this whole clause away? Undefined behaviour?
> 
> Anyway, I am not really sure we need to argue here, when the fix is
> dead easy ...

Agreed, It doesn't need to discuss about this.
If there is a potential problem, it needs to fix it.

Best Regards,
Jaehoon Chung

> 
> Cheers,
> Andre
> 
>> I think that's why the compiler doesn't warn. 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>   
>>>> that is why there is no warning.
>>>> In order to test , printed out mode value which suggests this variable is
>>>> initialized.  
>>>
>>> To what? Just because you printed 0(?) in your test doesn't mean that
>>> this will always be the case.
>>>
>>> Cheers,
>>> Andre
>>>   
>>
> 
>