On Mon, Apr 30, 2018 at 08:51:45AM +1000, NeilBrown wrote:
> On Sun, Apr 29 2018, Christian Lütke-Stetzkamp wrote:
>
> > The function msdc_set_blknum consists of one (real) line of code and
> > is only called once, so inline it makes the code shorter and more
> > readable.
> >
> > Signed-off-by: Christian Lütke-Stetzkamp <christian@xxxxxxxx>
> > ---
> > drivers/staging/mt7621-mmc/sd.c | 10 +---------
> > 1 file changed, 1 insertion(+), 9 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> > index c9f0180f3220..00c862b02419 100644
> > --- a/drivers/staging/mt7621-mmc/sd.c
> > +++ b/drivers/staging/mt7621-mmc/sd.c
> > @@ -1182,14 +1182,6 @@ static void msdc_dma_setup(struct msdc_host *host, struct msdc_dma *dma,
> > msdc_dma_config(host, dma);
> > }
> >
> > -/* set block number before send command */
> > -static void msdc_set_blknum(struct msdc_host *host, u32 blknum)
> > -{
> > - void __iomem *base = host->base;
> > -
> > - sdr_write32(SDC_BLK_NUM, blknum);
> > -}
> > -
>
> This change is correct, but the reason isn't obvious.
> In mt6575_sd.h are:
>
> #define SDC_BLK_NUM REG_ADDR(SDC_BLK_NUM)
>
> #define REG_ADDR(x) (base + OFFSET_##x)
>
> So the above becomes
>
> sdr_write32((base + OFFSET_SDC_BLK_NUM), blknum);
>
> so the base is important.
>
> msdc_do_request() starts with
>
> void __iomem *base = host->base;
>
> and never changes host. So that "base" that is used in the new code is
> the same as the "base" used in the old code.
>
> Ugh.. what a horrible way to write code!
Yes it is horrible and in a future cleanup I will remove all the
things depending on that "magic" base variable. But currently this is
the common (bad) idiom all over this driver and I would like to
address them all at once.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
