RE: [PATCH v3 1/1] mmc: dw_mmc: Add IDMAC 64-bit address mode support

[Seungwon Jeon <tgih.jun@samsung.com>]

On Tue, August 20, 2013, Prabu Thangamuthu wrote:
> Hi Arnd, Seungwon Jeon,
> 
> > On Tue, August 20, 2013, Seungwon Jeon wrote:
> > Hi Prabu,
> >
> > On Tue, August 20, 2013, Arnd Bergmann wrote:
> > > On Monday 19 August 2013, Prabu Thangamuthu wrote:
> > > >
> > > > +#ifdef CONFIG_MMC_DW_IDMAC_64BIT_ADDRESS
> > > > +       u32     des1;   /* Reserved */
> > > > +
> > > > +       u32     des2;   /* Buffer sizes */
> > > > +#define IDMAC_SET_BUFFER1_SIZE(d, s) \
> > > > +       ((d)->des2 = ((d)->des2 & 0x03ffe000) | ((s) & 0x1fff))
> > > > +
> > > > +       u32     des3;   /* Reserved */
> > > > +
> > > > +       u32     des4;   /* Lower 32-bits of Buffer Address Pointer 1*/
> > > > +       u32     des5;   /* Upper 32-bits of Buffer Address Pointer 1*/
> > > > +       u32     des6;   /* Lower 32-bits of Next Descriptor Address */
> > > > +       u32     des7;   /* Upper 32-bits of Next Descriptor Address */
> > > > +#else
> > > >         u32             des1;   /* Buffer sizes */
> > > >  #define IDMAC_SET_BUFFER1_SIZE(d, s) \
> > > >         ((d)->des1 = ((d)->des1 & 0x03ffe000) | ((s) & 0x1fff)) @@
> > > > -73,6 +86,7 @@ struct idmac_desc {
> > > >         u32             des2;   /* buffer 1 physical address */
> > > >
> > > >         u32             des3;   /* buffer 2 physical address */
> > > > +#endif /* CONFIG_MMC_DW_IDMAC_64BIT_ADDRESS */
> > >
> > > This is not a good idea: It means you cannot build the driver to
> > > support both 32 and 64 bit at run-time. You have to remove all the
> > > #ifdef here and replace it with runtime checks. You also need to
> > > update the binding document to provide a way to detect which one is
> > > present in a given system.
> 
> OK, I will modify the code to support both 32-bit and 64-bit at run-time based on
> hardware capability.
> 
> 
> > I guess HCON register can be used to identify 32 or 64 bit, right?
> > You already used it.
> > 	addr_config = (mci_readl(host, HCON) >> 27) & 0x01;
> >
> > Then, CONFIG_MMC_DW_IDMAC_64BIT_ADDRESS could be removed.
> 
> You are correct. HCON register 27th bit is used to decide 32-bit or 64-bit support.
> But, this bit is reserved in older IP versions and it is valid from IP version 2.70a onwards only.
Yes, 27th is reserved bit filed. If it's read in old version host, it could be '0' by reset value.
Then, we can decide that this host supports only 32-bit address anyway?
If right, we don't need to get version.


> So, I will consider both IP version and HCON register 27th bit to provide run-time support.
> 
> Thank you Arnd and Seungwon Jeo for your valuable comments.
> 
> 
Thanks,
Seungwon Jeon