RE: [PATCH/RFC v2] mmc: host: renesas_sdhi: Refactor of_device_id.data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Wolfram-san,

> From: Wolfram Sang, Sent: Wednesday, June 30, 2021 1:42 PM
> 
> Hi Shimoda-san,
> 
> thank you for taking care of this!

Thank you for your review!

> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> 
> I think this Rep-by can go. Test bot mentioned one build error of v1,
> but it didn't report that we should refactor this code.

You're correct. Perhaps, adding "# build fix on RFC" is better?
I checked the commit history, and I found such tags.

> >  -- should I make step-by-step patches to ease review?
> 
> I'd think this is good enough.

I got it.

> >  -- should I rename the current renesas_sdhi_of_data (e.g. renesas_sdhi_param)?
> >     (renesas_sdhi_of_data and renesas_sdhi_of_data_with_quirks seem strange
> >      a little?)
> 
> Yes, this may be a little better. I'd think the current naming is good
> enough, though.

I got it.

> >  - I tested this patch on r8a77951 (ES3.0), r8a77960 (ES1.0) and r8a77965.
> 
> I also tested this on r8a77965.

Thanks!

> >  - Also I tested this patch on r8a7791 [2].
> 
> I'll check r8a7790 later.

Thanks!

> >  static const struct renesas_sdhi_quirks sdhi_quirks_r8a7796_es13 = {
> >  	.hs400_4taps = true,
> >  	.hs400_bad_taps = BIT(2) | BIT(3) | BIT(6) | BIT(7),
> >  	.hs400_calib_table = r8a7796_es13_calib_table,
> >  };
> 
> You leave the quirk handling of different ES versions still in
> renesas_sdhi_core. I'd think this should also be moved to
> renesas_sdhi_internal_dmac? Then we have all the handling in one place.

I think so. So, I'll try this.

> > +static const struct renesas_sdhi_of_data_with_quirks of_r8a7795_compatible = {
> > +	.of_data	= &of_data_rcar_gen3,
> > +	.quirks		= &sdhi_quirks_bad_taps2367,
> > +};
> 
> I'd suggest only a single space (and not a tabulator) before the "=".

I got it.

> Rest looks good so far!

Thank you for your review!

Best regards,
Yoshihiro Shimoda





[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux