From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751340AbcAVFEL (ORCPT ); Fri, 22 Jan 2016 00:04:11 -0500 Received: from mailout1.samsung.com ([203.254.224.24]:34275 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750759AbcAVFEG (ORCPT ); Fri, 22 Jan 2016 00:04:06 -0500 X-AuditID: cbfee68e-f793c6d00000136c-1b-56a1b843fe96 Message-id: <56A1B843.7030603@samsung.com> Date: Fri, 22 Jan 2016 14:04:03 +0900 From: Jaehoon Chung User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-version: 1.0 To: Shawn Lin , Ulf Hansson Cc: linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH] mmc: dw_mmc: remove redundant num_slots check References: <1453362769-16223-1-git-send-email-shawn.lin@rock-chips.com> <56A19811.9000101@samsung.com> <56A19CDE.3090401@rock-chips.com> In-reply-to: <56A19CDE.3090401@rock-chips.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupgkeLIzCtJLcpLzFFi42JZI2JSrOuyY2GYwdMGJovLu+awWRz5389o cefJelaL42vDHVg87lzbw+bxd9Z+Fo/Pm+QCmKO4bFJSczLLUov07RK4Mk7Ouc1W8FisYvbl hAbGW4JdjJwcEgImEmcnbmWBsMUkLtxbz9bFyMUhJLCCUWLP0rWsMEUfrjxggkgsZZR49uMo lPOAUWJTyw9GkCpeAS2JlgcTwEaxCKhKzP26kA3EZhPQkdj+7TgTiC0qECbxYN1eVoh6QYkf k++B1YsI+Els3bEVrJ5ZwFri549WsBphATeJtTtbmSGWdTNK9CxYDZbgFNCTmPP7ClAzB1CD nsT9i1oQvfISm9e8BauXEFjFLvG//RojxEECEt8mHwKrlxCQldh0gBniM0mJgytusExgFJuF 5KRZCFNnIZm6gJF5FaNoakFyQXFSepGRXnFibnFpXrpecn7uJkZgBJ3+96xvB+PNA9aHGAU4 GJV4eDl0F4YJsSaWFVfmHmI0BTpiIrOUaHI+ME7zSuINjc2MLExNTI2NzC3NlMR5E6R+BgsJ pCeWpGanphakFsUXleakFh9iZOLglGpg5NzBvaksnPvmbY7+9/GBR8qm/5b/u1uD4Srnb4M/ LGnX/X0ile0aFkUultH0XvXWubNlXk/e9d/WZ94XxCy7ZdCgKL5T6MA7pr98t7b92lDdo9/I 4sv1I3DmRxuep+cOf7r6WvHzW4P11xo4Ji/z/SzGkW2U0P5DgvOFiGew23QO1SWpXC8TlViK MxINtZiLihMBeuzd5ZsCAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrKIsWRmVeSWpSXmKPExsVy+t9jAV3nHQvDDCZNtLK4vGsOm8WR//2M FneerGe1OL423IHF4861PWwef2ftZ/H4vEkugDmqgdEmIzUxJbVIITUvOT8lMy/dVsk7ON45 3tTMwFDX0NLCXEkhLzE31VbJxSdA1y0zB2ibkkJZYk4pUCggsbhYSd8O04TQEDddC5jGCF3f kCC4HiMDNJCwhjHj5JzbbAWPxSpmX05oYLwl2MXIySEhYCLx4coDJghbTOLCvfVsXYxcHEIC Sxklnv04ygThPGCU2NTygxGkildAS6LlwQQWEJtFQFVi7teFbCA2m4COxPZvx8EmiQqESTxY t5cVol5Q4sfke2D1IgJ+Elt3bAWrZxawlvj5oxWsRljATWLtzlZmiGXdjBI9C1aDJTgF9CTm /L4C1MwB1KAncf+iFkSvvMTmNW+ZJzAKzEKyYhZC1SwkVQsYmVcxSqQWJBcUJ6XnGuallusV J+YWl+al6yXn525iBEfpM6kdjAd3uR9iFOBgVOLh5dRdGCbEmlhWXJl7iFGCg1lJhDdsMlCI NyWxsiq1KD++qDQntfgQoykwDCYyS4km5wMTSF5JvKGxiZmRpZG5oYWRsbmSOG/tpcgwIYH0 xJLU7NTUgtQimD4mDk6pBsbESydfzYg+7J6xcPujGyncasJVm1dH7N/4edvLkM2iSvLPWIL+ xk/uKNpk8XrC2h/TOLXazvBz6K9Ri3+z42BK8Km00DPzu2O/b96T6pPiXOdX3cP6pNdiV/f6 dhfxyb71ZjfC0n9vu89x5rTnT7k5j25u4cnRdblrfd25IlZhyQLHU3ce5yYosRRnJBpqMRcV JwIAOPwsiegCAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/22/2016 12:07 PM, Shawn Lin wrote: > On 2016/1/22 10:46, Jaehoon Chung wrote: >> Hi, Shawn. >> >> On 01/21/2016 04:52 PM, Shawn Lin wrote: >>> num_slots comes from pdata if existing, otherwise from >>> dw_mci_parse_dt which make it at least one slot. If >>> num_slots is less than 1 for the existing pdata case, >>> current code return -ENODEV. But dw_mci_probe seems to >>> treat this a optional case as it will call SDMMC_GET_SLOT_NUM >>> if no slot assigned. >> >> Well, we need to consider more thing.. >> Host can get the number of slot from SDMMC_GET_SLOT_NUM(). >> But i think this way also has the problem. >> >> num_slot isn't defined anywhere, and num_slot should be set to value of SDMMC_GET_SLOT_NUM. >> If that value is higher than 1, it should be blocking..(I didn't test all cases..) >> > > Actually, from the code itself, it confused me the way about how we get > num_slot. At leaset, we might should try to cleanup it someway to make > it a little more clear. And just as what you point out, we see some > broblem here. > >> Even though this patch is not correct, i could check the problem relevant to num_slot, because of this patch. :) >> > > Nice to here that. I make it a RFC patch since I also not quite sure > about all cases including some corner cases. Let's think it twice. > >> my suggestion is if pdata->num_slot is not defined anywhere, just set to 1 by default. >> not take from SDMMC_GET_SLOT_NUM. >> > > yes, SDMMC_GET_SLOT_NUM is the capability of controller, num_slot is > hardware wired number. So, geting it from SDMMC_GET_SLOT_NUM has > problem. > >> if (host->pdata->nums_slots < 1 || >> host->pdata->nums_slots > SDMMC_GET_SLOT_NUM()) >> >> This is correct condition. num_slots can't be higher than number of supported slots. >> how about? > > Seems reasonable. > I guess you want to come up with a new patch dealing with it? :) No matter who does this. If you are ok, i will wait for patch. :) Best Regards, Jaehoon Chung > >> >> Best Regards, >> Jaehoon Chung >> >>> >>> Signed-off-by: Shawn Lin >>> >>> --- >>> >>> drivers/mmc/host/dw_mmc.c | 6 ------ >>> 1 file changed, 6 deletions(-) >>> >>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >>> index 7128351..a116ec6 100644 >>> --- a/drivers/mmc/host/dw_mmc.c >>> +++ b/drivers/mmc/host/dw_mmc.c >>> @@ -2949,12 +2949,6 @@ int dw_mci_probe(struct dw_mci *host) >>> } >>> } >>> >>> - if (host->pdata->num_slots < 1) { >>> - dev_err(host->dev, >>> - "Platform data must supply num_slots.\n"); >>> - return -ENODEV; >>> - } >>> - >>> host->biu_clk = devm_clk_get(host->dev, "biu"); >>> if (IS_ERR(host->biu_clk)) { >>> dev_dbg(host->dev, "biu clock not available\n"); >>> >> >> >> >> > >