linux-kernel - RE: [PATCH v6] mmc: dw_mmc: Add IDMAC 64-bit address mode support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <705D14B1C7978B40A723277C067CEDE21B333F60@IN01WEMBXB.internal.synopsys.com>
Date:	Thu, 16 Oct 2014 12:01:26 +0000
From:	Prabu Thangamuthu <Prabu.T@...opsys.com>
To:	Jaehoon Chung <jh80.chung@...sung.com>,
	Vivek Gautam <gautam.vivek@...sung.com>,
	Prabu Thangamuthu <Prabu.T@...opsys.com>
CC:	Alim Akhtar <alim.akhtar@...il.com>,
	Seungwon Jeon <tgih.jun@...sung.com>,
	Chris Ball <chris@...ntf.net>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Manjunath M Bettegowda" <Manjunath.MB@...opsys.com>
Subject: RE: [PATCH v6] mmc: dw_mmc: Add IDMAC 64-bit address mode support

Hi,

On Thu, Oct 16, 2014 at 4:42 PM, Jaehoon Chung <jh80.chung@...sung.com> wrote:
> On 10/16/2014 08:09 PM, Vivek Gautam wrote:
> > Hi Prabu,
> >
> >
> > On Thu, Oct 16, 2014 at 4:09 PM, Prabu Thangamuthu
> <Prabu.T@...opsys.com> wrote:
> >>
> >> Hi Jaehoon, Vivek, Alim,
> >>
> >> Thanks for your review comments.
> >>
> >> On Thu, Oct 16, 2014 at 12:20 PM, Jaehoon Chung
> <jh80.chung@...sung.com> wrote:
> >>> Hi, Prabu.
> >>>
> >>> On 10/15/2014 09:05 PM, Vivek Gautam wrote:
> >>>> Hi Prabu,
> >>>>
> >>>>
> >>>> On Tue, Oct 14, 2014 at 5:41 PM, Alim Akhtar
> >>>> <alim.akhtar@...il.com>
> >>> wrote:
> >>>>> Hi Prahu,
> >>>>> Thanks for a quick re-spin o the patch.
> >>>>> One last comment, this is more of a information seek.
> >>>>> On Thu, Oct 9, 2014 at 1:09 PM, Prabu Thangamuthu
> >>> <Prabu.T@...opsys.com> wrote:
> >>>>>> Synopsys DW_MMC IP core supports Internal DMA Controller with
> >>>>>> 64-bit
> >>> address mode from IP version 2.70a onwards.
> >>>>>> Updated the driver to support IDMAC 64-bit addressing mode.
> >>>>>>
> >>>>>> Tested the features in DW_MMC core v2.70a and v2.40a with HAPS-
> 51
> >>> setup and driver is working fine.
> >>>>>>
> >>>>>> Signed-off-by: Prabu Thangamuthu <prabu.t@...opsys.com>
> >>>>>> ---
> >>>>>> Change log v6:
> >>>>>>         - Updated with review comment.
> >>>>>>
> >>>>>> Change log v5:
> >>>>>>         - Recreated the patch against linux-next as this patch is
> >>>>>> required for another patch
> >>>>>> http://www.spinics.net/lists/arm-kernel/msg357985.html
> >>>>>>
> >>>>>> Change log v4:
> >>>>>>         - Add the dynamic support for 32-bit and 64-bit address
> >>>>>> mode based
> >>> on hw configuration.
> >>>>>>         - Removed the CONFIG_MMC_DW_IDMAC_64BIT_ADDRESS
> macro.
> >>>>>>
> >>>>>>  drivers/mmc/host/dw_mmc.c  |  195
> >>> +++++++++++++++++++++++++++++++++++---------
> >>>>>>  drivers/mmc/host/dw_mmc.h  |   11 +++
> >>>>>>  include/linux/mmc/dw_mmc.h |    2 +
> >>>>>>  3 files changed, 170 insertions(+), 38 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/mmc/host/dw_mmc.c
> >>> b/drivers/mmc/host/dw_mmc.c
> >>>>>> index 69f0cc6..c3b75a5 100644
> >>>>>> --- a/drivers/mmc/host/dw_mmc.c
> >>>>>> +++ b/drivers/mmc/host/dw_mmc.c
> >>>>>> @@ -62,6 +62,24 @@
> >>>>>>                                  SDMMC_IDMAC_INT_FBE |
> SDMMC_IDMAC_INT_RI | \
> >>>>>>                                  SDMMC_IDMAC_INT_TI)
> >>>>>>
> >>>>>> +struct idmac_desc_64addr {
> >>>>>> +       u32             des0;   /* Control Descriptor */
> >>>>>> +
> >>>>>> +       u32             des1;   /* Reserved */
> >>>>> What is the default value of these _Reserved_ descriptors? As
> >>>>> these are pointers, do you thing initializing then to zero make sense?
> >>
> >> We don't think it's required to assign zero to reserved des field as
> mobster core not going to use this field's for any operations unless user has
> modified it.
> >> Also it's working fine in our test set-up without initializing them to zero.
> 
> Right, it's not required to assign zero to there. And It's working fine, because
> it's not used or referred anywhere.
> Just Recommend to initialize to zero.

Fine.
 
> Actually, i don't understand why des1 is reserved..

It's reserved for future enhancement.

> >>
> >>>>
> >>>> I think the default reset value of these descriptors will depend on
> >>>> how dwmmc is integrated in h/w and how these descriptors are then
> >>>> configured.
> >>>>
> >>>> So it makes sense to initialize these descriptors to zero before use.
> >>>> We have seen on our exynos7 system, that we need to initialize the
> >>>> descriptors des1 and des2 to zero before we use them further.
> >>
> >> It looks like exynos7 is using this reserved fields for some other purposes.
> To make it generic, we will initialize the reserved fields to zero.
> >>
> >> Does exynos7 requires des2 (Buffer sizes) also to be initialized to zero?
> >
> > Yes, we need des2 to be initialized to zero.
> > One question however, do we have some default reset value set to these
> > descriptors ? If yes then how can we check this value ?
> 
> I knew it doesn't have the reset value for descriptors.
> 

Yes, it's correct.


Regards,
Prabu Thangamuthu.