On Sun, Apr 29 2018, Christian Lütke-Stetzkamp wrote: > Currently sdr_get_field is a macro, to bring the code in line with the > upstream driver mtk-sd, it is changed to a function. > > Signed-off-by: Christian Lütke-Stetzkamp <[email protected]> > --- > drivers/staging/mt7621-mmc/mt6575_sd.h | 10 +++++----- > drivers/staging/mt7621-mmc/sd.c | 14 ++++++++------ > 2 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/staging/mt7621-mmc/mt6575_sd.h > b/drivers/staging/mt7621-mmc/mt6575_sd.h > index 90b4ee6b396b..195c7333f77d 100644 > --- a/drivers/staging/mt7621-mmc/mt6575_sd.h > +++ b/drivers/staging/mt7621-mmc/mt6575_sd.h > @@ -980,10 +980,10 @@ static inline void sdr_set_field(void __iomem *reg, u32 > field, u32 val) > writel(tv, reg); > } > > -#define sdr_get_field(reg, field, val) > \ > -do { \ > - volatile unsigned int tv = sdr_read32(reg); \ > - val = ((tv & (field)) >> (ffs((unsigned int)field) - 1)); \ > -} while (0) > +static inline void sdr_get_field(void __iomem *reg, u32 field, u32 *val) > +{ > + unsigned int tv = readl(reg); > + *val = ((tv & field) >> (ffs((unsigned int)field) - 1)); > +}
Converting this to a function it good, but it should return val rather
than set *val.
>
> #endif
> diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> index 00dacf8c9625..f4da48a5772b 100644
> --- a/drivers/staging/mt7621-mmc/sd.c
> +++ b/drivers/staging/mt7621-mmc/sd.c
> @@ -1577,8 +1577,8 @@ static int msdc_tune_cmdrsp(struct msdc_host *host,
> struct mmc_command *cmd)
> ==========================*/
>
> // save the previous tune result
> - sdr_get_field(MSDC_IOCON, MSDC_IOCON_RSPL, orig_rsmpl);
> - sdr_get_field(MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRRDLY, orig_rrdly);
> + sdr_get_field(MSDC_IOCON, MSDC_IOCON_RSPL, &orig_rsmpl);
> + sdr_get_field(MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRRDLY, &orig_rrdly);
So this becomes
> + orig_rsmpl = sdr_get_field(MSDC_IOCON, MSDC_IOCON_RSPL);
> + orig_rrdly = sdr_get_field(MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRRDLY);
which is much easier to understand at first glance.
Thanks,
NeilBrown
>
> rrdly = 0;
> do {
> @@ -1640,7 +1640,7 @@ static int msdc_tune_bread(struct mmc_host *mmc, struct
> mmc_request *mrq)
> int result = -1;
> u32 skip = 1;
>
> - sdr_get_field(MSDC_IOCON, MSDC_IOCON_DSPL, orig_dsmpl);
> + sdr_get_field(MSDC_IOCON, MSDC_IOCON_DSPL, &orig_dsmpl);
>
> /* Tune Method 2. */
> sdr_set_field(MSDC_IOCON, MSDC_IOCON_DDLSEL, 1);
> @@ -1664,7 +1664,9 @@ static int msdc_tune_bread(struct mmc_host *mmc, struct
> mmc_request *mrq)
> }
> result = msdc_do_request(mmc, mrq);
>
> - sdr_get_field(SDC_DCRC_STS, SDC_DCRC_STS_POS |
> SDC_DCRC_STS_NEG, dcrc); /* RO */
> + sdr_get_field(SDC_DCRC_STS,
> + SDC_DCRC_STS_POS | SDC_DCRC_STS_NEG,
> + &dcrc); /* RO */
> if (!ddr)
> dcrc &= ~SDC_DCRC_STS_NEG;
> ERR_MSG("TUNE_BREAD<%s> dcrc<0x%x>
> DATRDDLY0/1<0x%x><0x%x> dsmpl<0x%x>",
> @@ -1751,8 +1753,8 @@ static int msdc_tune_bwrite(struct mmc_host *mmc,
> struct mmc_request *mrq)
>
> // MSDC_IOCON_DDR50CKD need to check. [Fix me]
>
> - sdr_get_field(MSDC_PAD_TUNE, MSDC_PAD_TUNE_DATWRDLY, orig_wrrdly);
> - sdr_get_field(MSDC_IOCON, MSDC_IOCON_DSPL, orig_dsmpl);
> + sdr_get_field(MSDC_PAD_TUNE, MSDC_PAD_TUNE_DATWRDLY, &orig_wrrdly);
> + sdr_get_field(MSDC_IOCON, MSDC_IOCON_DSPL, &orig_dsmpl);
>
> /* Tune Method 2. just DAT0 */
> sdr_set_field(MSDC_IOCON, MSDC_IOCON_DDLSEL, 1);
> --
> 2.16.1
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list [email protected] http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
