Re: [PATCH v24 11/15] mmc: renesas_sdhi: Add RZ/G3L SDHI support

[wsa+renesas <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]


> > So initially bit 8+9 will have a value. This will be moved to bit 16
> > + 17 first Then we clear the bits 8 + 9. Bit8(SCLKEN) and
> > Bit9(SDCLKOFFEN)

Okay, I get it now. Thanks for the explanation.

> > > Does RZG3L have a dedicated naming? I'd guess *_HWADJ1. I wonder if it
> > > makes sense to have a dedicated define for it. I tend to think so.
> > 
> > There is no *_HWADJ1 mentioned in HW manual.
> > Hardware manual mention this register as "SDm_SCC_HWADJ2"

OK.

> > 
> > >
> > > > +		if (hwadj2)
> > > > +			sd_scc_write32(host, priv, RZG3L_SDHI_SCC_HWADJ2, 0x3FFF);
> > > > +	} else {
> > > >  		sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2, 0x1);
> > > > +		if (hwadj2)
> > > > +			sd_scc_write32(host, priv, RZG3L_SDHI_SCC_HWADJ2, 0xFF);
> > >
> > > Magic values?
> > 
> > HW manual mentions the following,
> > 
> > HWADJ2[15:0] 1FFh Hardware Adjustment.
> > For 1.8V operation, specify 00FFh.
> > For 3.3V operation, specify 3FFFh.
> > 
> > Is the below macros ok for representing magic values??
> > 
> > SD_SCC_HWADJ2_1_8V 0xFF
> > SD_SCC_HWADJ2_3_3V 0x3FFF

If you replace the 'SD_'-prefix with 'RZG3L_SDHI_', then yes :)

> > > I wonder how likely it is that a SDHI variant has either only this or
> > > only TUNING_DELAY. Do you know of one? What would you think about merging the two? You know the RZ SDHI
> > much better...
> > 
> > HWADJ2 present only on RZ/G3L,
> > whereas TUNING_DELAY present on both RZ/G2L and RZ/G3L.

Ok, no merging then. Was worth asking.

One more thing I noticed. I think you can drop 'sdhi_quirks_rzg3l' in
favor of 'sdhi_quirks_fixed_addr', or?


Good luck hacking the updates. And thanks for the patience!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]