[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