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

Andre Przywara andre.przywara at arm.com
Tue Mar 2 00:04:26 CET 2021


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 ...

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
> >   
> 



More information about the U-Boot mailing list