* [PATCH V3] mmc: sdhci-msm: Reset GCC_SDCC_BCR register for SDHC
@ 2022-03-31 7:03 Shaik Sajida Bhanu
2022-03-31 18:42 ` Bjorn Andersson
0 siblings, 1 reply; 4+ messages in thread
From: Shaik Sajida Bhanu @ 2022-03-31 7:03 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, robh+dt
Cc: quic_asutoshd, quic_rampraka, quic_pragalla, quic_sartgarg,
quic_nitirawa, quic_sayalil, agross, bjorn.andersson,
krzysztof.kozlowski, linux-arm-msm, devicetree, linux-kernel,
Shaik Sajida Bhanu
Reset GCC_SDCC_BCR register before every fresh initilazation. This will
reset whole SDHC-msm controller, clears the previous power control
states and avoids, software reset timeout issues as below.
[ 5.458061][ T262] mmc1: Reset 0x1 never completed.
[ 5.462454][ T262] mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
[ 5.469065][ T262] mmc1: sdhci: Sys addr: 0x00000000 | Version: 0x00007202
[ 5.475688][ T262] mmc1: sdhci: Blk size: 0x00000000 | Blk cnt: 0x00000000
[ 5.482315][ T262] mmc1: sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
[ 5.488927][ T262] mmc1: sdhci: Present: 0x01f800f0 | Host ctl: 0x00000000
[ 5.495539][ T262] mmc1: sdhci: Power: 0x00000000 | Blk gap: 0x00000000
[ 5.502162][ T262] mmc1: sdhci: Wake-up: 0x00000000 | Clock: 0x00000003
[ 5.508768][ T262] mmc1: sdhci: Timeout: 0x00000000 | Int stat: 0x00000000
[ 5.515381][ T262] mmc1: sdhci: Int enab: 0x00000000 | Sig enab: 0x00000000
[ 5.521996][ T262] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
[ 5.528607][ T262] mmc1: sdhci: Caps: 0x362dc8b2 | Caps_1: 0x0000808f
[ 5.535227][ T262] mmc1: sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
[ 5.541841][ T262] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]: 0x00000000
[ 5.548454][ T262] mmc1: sdhci: Resp[2]: 0x00000000 | Resp[3]: 0x00000000
[ 5.555079][ T262] mmc1: sdhci: Host ctl2: 0x00000000
[ 5.559651][ T262] mmc1: sdhci_msm: ----------- VENDOR REGISTER DUMP-----------
[ 5.566621][ T262] mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg: 0x6000642c | DLL cfg2: 0x0020a000
[ 5.575465][ T262] mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl: 0x00010800 | DDR cfg: 0x80040873
[ 5.584658][ T262] mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xf88218a8 Vndr func3: 0x02626040
Fixes: 0eb0d9f4de34 ("mmc: sdhci-msm: Initial support for Qualcomm chipsets")
Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
---
Changes since V2:
- Dropped new line after fixes tag as suggested by Bjorn
Andersson.
- Passed device structure instead of passing platform_device
structure as a argument for sdhci_msm_gcc_reset() as suggested
by Bjorn Andersson.
- Replaced dev_err() with dev_err_probe() as suggested by Bjorn
Andersson.
Changes since V1:
- Added fixes tag as suggested by Ulf Hansson.
- Replaced devm_reset_control_get() with
devm_reset_control_get_optional_exclusive() as suggested by
Ulf Hansson.
---
drivers/mmc/host/sdhci-msm.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 50c71e0..e15e789 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -17,6 +17,7 @@
#include <linux/regulator/consumer.h>
#include <linux/interconnect.h>
#include <linux/pinctrl/consumer.h>
+#include <linux/reset.h>
#include "sdhci-pltfm.h"
#include "cqhci.h"
@@ -284,6 +285,7 @@ struct sdhci_msm_host {
bool uses_tassadar_dll;
u32 dll_config;
u32 ddr_config;
+ struct reset_control *core_reset;
bool vqmmc_enabled;
};
@@ -2482,6 +2484,39 @@ static inline void sdhci_msm_get_of_property(struct platform_device *pdev,
of_property_read_u32(node, "qcom,dll-config", &msm_host->dll_config);
}
+static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+ int ret = 0;
+
+ msm_host->core_reset = devm_reset_control_get_optional_exclusive(dev, "core_reset");
+ if (IS_ERR(msm_host->core_reset))
+ return dev_err_probe(dev, PTR_ERR(msm_host->core_reset),
+ "unable to acquire core_reset\n");
+
+ if (!msm_host->core_reset)
+ return 0;
+
+ ret = reset_control_assert(msm_host->core_reset);
+ if (ret)
+ return dev_err_probe(dev, ret, "core_reset assert failed\n");
+
+ /*
+ * The hardware requirement for delay between assert/deassert
+ * is at least 3-4 sleep clock (32.7KHz) cycles, which comes to
+ * ~125us (4/32768). To be on the safe side add 200us delay.
+ */
+ usleep_range(200, 210);
+
+ ret = reset_control_deassert(msm_host->core_reset);
+ if (ret)
+ return dev_err_probe(dev, ret, "core_reset deassert failed\n");
+
+ usleep_range(200, 210);
+
+ return 0;
+}
static int sdhci_msm_probe(struct platform_device *pdev)
{
@@ -2529,6 +2564,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
+ ret = sdhci_msm_gcc_reset(&pdev->dev, host);
+ if (ret)
+ goto pltfm_free;
+
/* Setup SDCC bus voter clock. */
msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
if (!IS_ERR(msm_host->bus_clk)) {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH V3] mmc: sdhci-msm: Reset GCC_SDCC_BCR register for SDHC
2022-03-31 7:03 [PATCH V3] mmc: sdhci-msm: Reset GCC_SDCC_BCR register for SDHC Shaik Sajida Bhanu
@ 2022-03-31 18:42 ` Bjorn Andersson
2022-04-06 9:46 ` Sajida Bhanu (Temp)
0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Andersson @ 2022-03-31 18:42 UTC (permalink / raw)
To: Shaik Sajida Bhanu
Cc: adrian.hunter, ulf.hansson, robh+dt, quic_asutoshd,
quic_rampraka, quic_pragalla, quic_sartgarg, quic_nitirawa,
quic_sayalil, agross, krzysztof.kozlowski, linux-arm-msm,
devicetree, linux-kernel
On Thu 31 Mar 00:03 PDT 2022, Shaik Sajida Bhanu wrote:
> Reset GCC_SDCC_BCR register before every fresh initilazation. This will
> reset whole SDHC-msm controller, clears the previous power control
> states and avoids, software reset timeout issues as below.
>
> [ 5.458061][ T262] mmc1: Reset 0x1 never completed.
> [ 5.462454][ T262] mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
> [ 5.469065][ T262] mmc1: sdhci: Sys addr: 0x00000000 | Version: 0x00007202
> [ 5.475688][ T262] mmc1: sdhci: Blk size: 0x00000000 | Blk cnt: 0x00000000
> [ 5.482315][ T262] mmc1: sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
> [ 5.488927][ T262] mmc1: sdhci: Present: 0x01f800f0 | Host ctl: 0x00000000
> [ 5.495539][ T262] mmc1: sdhci: Power: 0x00000000 | Blk gap: 0x00000000
> [ 5.502162][ T262] mmc1: sdhci: Wake-up: 0x00000000 | Clock: 0x00000003
> [ 5.508768][ T262] mmc1: sdhci: Timeout: 0x00000000 | Int stat: 0x00000000
> [ 5.515381][ T262] mmc1: sdhci: Int enab: 0x00000000 | Sig enab: 0x00000000
> [ 5.521996][ T262] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
> [ 5.528607][ T262] mmc1: sdhci: Caps: 0x362dc8b2 | Caps_1: 0x0000808f
> [ 5.535227][ T262] mmc1: sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
> [ 5.541841][ T262] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]: 0x00000000
> [ 5.548454][ T262] mmc1: sdhci: Resp[2]: 0x00000000 | Resp[3]: 0x00000000
> [ 5.555079][ T262] mmc1: sdhci: Host ctl2: 0x00000000
> [ 5.559651][ T262] mmc1: sdhci_msm: ----------- VENDOR REGISTER DUMP-----------
> [ 5.566621][ T262] mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg: 0x6000642c | DLL cfg2: 0x0020a000
> [ 5.575465][ T262] mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl: 0x00010800 | DDR cfg: 0x80040873
> [ 5.584658][ T262] mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xf88218a8 Vndr func3: 0x02626040
>
> Fixes: 0eb0d9f4de34 ("mmc: sdhci-msm: Initial support for Qualcomm chipsets")
> Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
> ---
>
> Changes since V2:
> - Dropped new line after fixes tag as suggested by Bjorn
> Andersson.
> - Passed device structure instead of passing platform_device
> structure as a argument for sdhci_msm_gcc_reset() as suggested
> by Bjorn Andersson.
> - Replaced dev_err() with dev_err_probe() as suggested by Bjorn
> Andersson.
Thanks, looks much better. Still some things I would like to see
improved below.
> Changes since V1:
> - Added fixes tag as suggested by Ulf Hansson.
> - Replaced devm_reset_control_get() with
> devm_reset_control_get_optional_exclusive() as suggested by
> Ulf Hansson.
> ---
> drivers/mmc/host/sdhci-msm.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 50c71e0..e15e789 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -17,6 +17,7 @@
> #include <linux/regulator/consumer.h>
> #include <linux/interconnect.h>
> #include <linux/pinctrl/consumer.h>
> +#include <linux/reset.h>
>
> #include "sdhci-pltfm.h"
> #include "cqhci.h"
> @@ -284,6 +285,7 @@ struct sdhci_msm_host {
> bool uses_tassadar_dll;
> u32 dll_config;
> u32 ddr_config;
> + struct reset_control *core_reset;
> bool vqmmc_enabled;
> };
>
> @@ -2482,6 +2484,39 @@ static inline void sdhci_msm_get_of_property(struct platform_device *pdev,
> of_property_read_u32(node, "qcom,dll-config", &msm_host->dll_config);
> }
>
> +static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + int ret = 0;
First use of this variable is an assignment, so no need to initialize it
here.
> +
> + msm_host->core_reset = devm_reset_control_get_optional_exclusive(dev, "core_reset");
reset-names will only be used to identify resets and hence there's no
reason to include "_reset" in the identifier.
If this is the only reset for the controller, there's actually no reason
for identifying it, you can omit reset-names from the binding and just
pass NULL here (to get the first resets = <>).
> + if (IS_ERR(msm_host->core_reset))
> + return dev_err_probe(dev, PTR_ERR(msm_host->core_reset),
> + "unable to acquire core_reset\n");
> +
> + if (!msm_host->core_reset)
> + return 0;
> +
> + ret = reset_control_assert(msm_host->core_reset);
> + if (ret)
> + return dev_err_probe(dev, ret, "core_reset assert failed\n");
> +
> + /*
> + * The hardware requirement for delay between assert/deassert
> + * is at least 3-4 sleep clock (32.7KHz) cycles, which comes to
> + * ~125us (4/32768). To be on the safe side add 200us delay.
> + */
> + usleep_range(200, 210);
> +
> + ret = reset_control_deassert(msm_host->core_reset);
> + if (ret)
> + return dev_err_probe(dev, ret, "core_reset deassert failed\n");
> +
> + usleep_range(200, 210);
> +
sdhci_msm_gcc_reset() is only called once during probe(), so there's no
reason to carry the reset_control pointer in struct sdhci_msm_host. Make
it a local variable and use reset_control_get_optional_exclusive() and
reset_control_put() the reset here before returning.
Regards,
Bjorn
> + return 0;
> +}
>
> static int sdhci_msm_probe(struct platform_device *pdev)
> {
> @@ -2529,6 +2564,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>
> msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>
> + ret = sdhci_msm_gcc_reset(&pdev->dev, host);
> + if (ret)
> + goto pltfm_free;
> +
> /* Setup SDCC bus voter clock. */
> msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
> if (!IS_ERR(msm_host->bus_clk)) {
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V3] mmc: sdhci-msm: Reset GCC_SDCC_BCR register for SDHC
2022-03-31 18:42 ` Bjorn Andersson
@ 2022-04-06 9:46 ` Sajida Bhanu (Temp)
0 siblings, 0 replies; 4+ messages in thread
From: Sajida Bhanu (Temp) @ 2022-04-06 9:46 UTC (permalink / raw)
To: Bjorn Andersson
Cc: adrian.hunter, ulf.hansson, robh+dt, quic_asutoshd,
quic_rampraka, quic_pragalla, quic_sartgarg, quic_nitirawa,
quic_sayalil, agross, krzysztof.kozlowski, linux-arm-msm,
devicetree, linux-kernel
On 4/1/2022 12:12 AM, Bjorn Andersson wrote:
> On Thu 31 Mar 00:03 PDT 2022, Shaik Sajida Bhanu wrote:
>
>> Reset GCC_SDCC_BCR register before every fresh initilazation. This will
>> reset whole SDHC-msm controller, clears the previous power control
>> states and avoids, software reset timeout issues as below.
>>
>> [ 5.458061][ T262] mmc1: Reset 0x1 never completed.
>> [ 5.462454][ T262] mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
>> [ 5.469065][ T262] mmc1: sdhci: Sys addr: 0x00000000 | Version: 0x00007202
>> [ 5.475688][ T262] mmc1: sdhci: Blk size: 0x00000000 | Blk cnt: 0x00000000
>> [ 5.482315][ T262] mmc1: sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
>> [ 5.488927][ T262] mmc1: sdhci: Present: 0x01f800f0 | Host ctl: 0x00000000
>> [ 5.495539][ T262] mmc1: sdhci: Power: 0x00000000 | Blk gap: 0x00000000
>> [ 5.502162][ T262] mmc1: sdhci: Wake-up: 0x00000000 | Clock: 0x00000003
>> [ 5.508768][ T262] mmc1: sdhci: Timeout: 0x00000000 | Int stat: 0x00000000
>> [ 5.515381][ T262] mmc1: sdhci: Int enab: 0x00000000 | Sig enab: 0x00000000
>> [ 5.521996][ T262] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
>> [ 5.528607][ T262] mmc1: sdhci: Caps: 0x362dc8b2 | Caps_1: 0x0000808f
>> [ 5.535227][ T262] mmc1: sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
>> [ 5.541841][ T262] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]: 0x00000000
>> [ 5.548454][ T262] mmc1: sdhci: Resp[2]: 0x00000000 | Resp[3]: 0x00000000
>> [ 5.555079][ T262] mmc1: sdhci: Host ctl2: 0x00000000
>> [ 5.559651][ T262] mmc1: sdhci_msm: ----------- VENDOR REGISTER DUMP-----------
>> [ 5.566621][ T262] mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg: 0x6000642c | DLL cfg2: 0x0020a000
>> [ 5.575465][ T262] mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl: 0x00010800 | DDR cfg: 0x80040873
>> [ 5.584658][ T262] mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xf88218a8 Vndr func3: 0x02626040
>>
>> Fixes: 0eb0d9f4de34 ("mmc: sdhci-msm: Initial support for Qualcomm chipsets")
>> Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
>> ---
>>
>> Changes since V2:
>> - Dropped new line after fixes tag as suggested by Bjorn
>> Andersson.
>> - Passed device structure instead of passing platform_device
>> structure as a argument for sdhci_msm_gcc_reset() as suggested
>> by Bjorn Andersson.
>> - Replaced dev_err() with dev_err_probe() as suggested by Bjorn
>> Andersson.
> Thanks, looks much better. Still some things I would like to see
> improved below.
Sure
>> Changes since V1:
>> - Added fixes tag as suggested by Ulf Hansson.
>> - Replaced devm_reset_control_get() with
>> devm_reset_control_get_optional_exclusive() as suggested by
>> Ulf Hansson.
>> ---
>> drivers/mmc/host/sdhci-msm.c | 39 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 50c71e0..e15e789 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -17,6 +17,7 @@
>> #include <linux/regulator/consumer.h>
>> #include <linux/interconnect.h>
>> #include <linux/pinctrl/consumer.h>
>> +#include <linux/reset.h>
>>
>> #include "sdhci-pltfm.h"
>> #include "cqhci.h"
>> @@ -284,6 +285,7 @@ struct sdhci_msm_host {
>> bool uses_tassadar_dll;
>> u32 dll_config;
>> u32 ddr_config;
>> + struct reset_control *core_reset;
>> bool vqmmc_enabled;
>> };
>>
>> @@ -2482,6 +2484,39 @@ static inline void sdhci_msm_get_of_property(struct platform_device *pdev,
>> of_property_read_u32(node, "qcom,dll-config", &msm_host->dll_config);
>> }
>>
>> +static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> + int ret = 0;
> First use of this variable is an assignment, so no need to initialize it
> here.
Sure
>> +
>> + msm_host->core_reset = devm_reset_control_get_optional_exclusive(dev, "core_reset");
> reset-names will only be used to identify resets and hence there's no
> reason to include "_reset" in the identifier.
>
> If this is the only reset for the controller, there's actually no reason
> for identifying it, you can omit reset-names from the binding and just
> pass NULL here (to get the first resets = <>).
ok Sure
>> + if (IS_ERR(msm_host->core_reset))
>> + return dev_err_probe(dev, PTR_ERR(msm_host->core_reset),
>> + "unable to acquire core_reset\n");
>> +
>> + if (!msm_host->core_reset)
>> + return 0;
>> +
>> + ret = reset_control_assert(msm_host->core_reset);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "core_reset assert failed\n");
>> +
>> + /*
>> + * The hardware requirement for delay between assert/deassert
>> + * is at least 3-4 sleep clock (32.7KHz) cycles, which comes to
>> + * ~125us (4/32768). To be on the safe side add 200us delay.
>> + */
>> + usleep_range(200, 210);
>> +
>> + ret = reset_control_deassert(msm_host->core_reset);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "core_reset deassert failed\n");
>> +
>> + usleep_range(200, 210);
>> +
> sdhci_msm_gcc_reset() is only called once during probe(), so there's no
> reason to carry the reset_control pointer in struct sdhci_msm_host. Make
> it a local variable and use reset_control_get_optional_exclusive() and
> reset_control_put() the reset here before returning.
>
> Regards,
> Bjorn
Sure Thanks for the review.
>> + return 0;
>> +}
>>
>> static int sdhci_msm_probe(struct platform_device *pdev)
>> {
>> @@ -2529,6 +2564,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>
>> msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>>
>> + ret = sdhci_msm_gcc_reset(&pdev->dev, host);
>> + if (ret)
>> + goto pltfm_free;
>> +
>> /* Setup SDCC bus voter clock. */
>> msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>> if (!IS_ERR(msm_host->bus_clk)) {
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V3] mmc: sdhci-msm: Reset GCC_SDCC_BCR register for SDHC
@ 2022-04-06 23:31 kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2022-04-06 23:31 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 18358 bytes --]
CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <1648710182-31899-1-git-send-email-quic_c_sbhanu@quicinc.com>
References: <1648710182-31899-1-git-send-email-quic_c_sbhanu@quicinc.com>
TO: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
TO: adrian.hunter(a)intel.com
TO: ulf.hansson(a)linaro.org
TO: robh+dt(a)kernel.org
CC: quic_asutoshd(a)quicinc.com
CC: quic_rampraka(a)quicinc.com
CC: quic_pragalla(a)quicinc.com
CC: quic_sartgarg(a)quicinc.com
CC: quic_nitirawa(a)quicinc.com
CC: quic_sayalil(a)quicinc.com
CC: agross(a)kernel.org
CC: bjorn.andersson(a)linaro.org
CC: krzysztof.kozlowski(a)canonical.com
CC: linux-arm-msm(a)vger.kernel.org
CC: devicetree(a)vger.kernel.org
CC: linux-kernel(a)vger.kernel.org
CC: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
Hi Shaik,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on ulf-hansson-mmc-mirror/next linux/master v5.18-rc1 next-20220406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Shaik-Sajida-Bhanu/mmc-sdhci-msm-Reset-GCC_SDCC_BCR-register-for-SDHC/20220331-150338
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 787af64d05cd528aac9ad16752d11bb1c6061bb9
:::::: branch date: 7 days ago
:::::: commit date: 7 days ago
config: riscv-randconfig-c006-20220405 (https://download.01.org/0day-ci/archive/20220407/202204070708.sE7Ed67N-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c4a1b07d0979e7ff20d7d541af666d822d66b566)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/667fad5561c9ebaeed5137640c6cbe75fa020a42
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Shaik-Sajida-Bhanu/mmc-sdhci-msm-Reset-GCC_SDCC_BCR-register-for-SDHC/20220331-150338
git checkout 667fad5561c9ebaeed5137640c6cbe75fa020a42
# save the config file to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
drivers/mmc/host/omap_hsmmc.c:821:2: note: Loop condition is false. Exiting loop
spin_lock_irqsave(&host->irq_lock, flags);
^
include/linux/spinlock.h:377:43: note: expanded from macro 'spin_lock_irqsave'
#define spin_lock_irqsave(lock, flags) \
^
drivers/mmc/host/omap_hsmmc.c:828:6: note: Assuming field 'data' is null
if (mrq->data && host->use_dma && dma_ch != -1)
^~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:828:16: note: Left side of '&&' is false
if (mrq->data && host->use_dma && dma_ch != -1)
^
drivers/mmc/host/omap_hsmmc.c:830:2: note: Null pointer value stored to field 'mrq'
host->mrq = NULL;
^~~~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:864:3: note: Returning from 'omap_hsmmc_request_done'
omap_hsmmc_request_done(host, data->mrq);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:1060:3: note: Returning from 'omap_hsmmc_xfer_done'
omap_hsmmc_xfer_done(host, data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:1074:4: note: Returning from 'omap_hsmmc_do_irq'
omap_hsmmc_do_irq(host, status);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:1076:7: note: Assuming the condition is false
if (status & CIRQ_EN)
^~~~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:1076:3: note: Taking false branch
if (status & CIRQ_EN)
^
drivers/mmc/host/omap_hsmmc.c:1072:2: note: Loop condition is true. Entering loop body
while (status & (INT_EN_MASK | CIRQ_EN)) {
^
drivers/mmc/host/omap_hsmmc.c:1073:7: note: Assuming field 'req_in_progress' is not equal to 0
if (host->req_in_progress)
^~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:1073:3: note: Taking true branch
if (host->req_in_progress)
^
drivers/mmc/host/omap_hsmmc.c:1074:4: note: Calling 'omap_hsmmc_do_irq'
omap_hsmmc_do_irq(host, status);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:1023:2: note: Taking false branch
dev_vdbg(mmc_dev(host->mmc), "IRQ Status is %x\n", status);
^
include/linux/dev_printk.h:261:2: note: expanded from macro 'dev_vdbg'
if (0) \
^
drivers/mmc/host/omap_hsmmc.c:1025:6: note: Assuming the condition is false
if (status & ERR_EN) {
^~~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:1025:2: note: Taking false branch
if (status & ERR_EN) {
^
drivers/mmc/host/omap_hsmmc.c:1057:6: note: 'end_cmd' is 0
if (end_cmd || ((status & CC_EN) && host->cmd))
^~~~~~~
drivers/mmc/host/omap_hsmmc.c:1057:6: note: Left side of '||' is false
drivers/mmc/host/omap_hsmmc.c:1057:19: note: Assuming the condition is true
if (end_cmd || ((status & CC_EN) && host->cmd))
^~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:1057:18: note: Left side of '&&' is true
if (end_cmd || ((status & CC_EN) && host->cmd))
^
drivers/mmc/host/omap_hsmmc.c:1057:38: note: Assuming field 'cmd' is non-null
if (end_cmd || ((status & CC_EN) && host->cmd))
^~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:1057:2: note: Taking true branch
if (end_cmd || ((status & CC_EN) && host->cmd))
^
drivers/mmc/host/omap_hsmmc.c:1058:3: note: Calling 'omap_hsmmc_cmd_done'
omap_hsmmc_cmd_done(host, host->cmd);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/omap_hsmmc.c:873:6: note: Access to field 'sbc' results in a dereference of a null pointer (loaded from field 'mrq')
if (host->mrq->sbc && (host->cmd == host->mrq->sbc) &&
^ ~~~
drivers/mmc/host/omap_hsmmc.c:945:8: warning: Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
len = sprintf(buf, "MMC IRQ 0x%x :", status);
^~~~~~~
drivers/mmc/host/omap_hsmmc.c:945:8: note: Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11
len = sprintf(buf, "MMC IRQ 0x%x :", status);
^~~~~~~
drivers/mmc/host/omap_hsmmc.c:950:10: warning: Call to function 'sprintf' is insecure as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
len = sprintf(buf, " %s", omap_hsmmc_status_bits[i]);
^~~~~~~
drivers/mmc/host/omap_hsmmc.c:950:10: note: Call to function 'sprintf' is insecure as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11
len = sprintf(buf, " %s", omap_hsmmc_status_bits[i]);
^~~~~~~
Suppressed 53 warnings (46 in non-user code, 7 with check filters).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
54 warnings generated.
drivers/mmc/host/sdhci-bcm-kona.c:222:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]
ret = 0;
^ ~
drivers/mmc/host/sdhci-bcm-kona.c:222:2: note: Value stored to 'ret' is never read
ret = 0;
^ ~
Suppressed 53 warnings (46 in non-user code, 7 with check filters).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
57 warnings generated.
>> drivers/mmc/host/sdhci-msm.c:257:8: warning: Excessive padding in 'struct sdhci_msm_host' (32 padding bytes, where 0 is optimal).
Optimal fields order:
pdev,
core_mem,
ice_mem,
bus_clk,
xo_clk,
clk_rate,
mmc,
var_ops,
offset,
core_reset,
bulk_clks,
pwr_irq_wait,
pwr_irq,
curr_pwr_state,
curr_io_level,
caps_0,
transfer_mode,
dll_config,
ddr_config,
use_14lpp_dll_reset,
tuning_done,
calibration_done,
saved_tuning_phase,
use_cdclp533,
pwr_irq_flag,
mci_removed,
restore_dll_config,
use_cdr,
updated_ddr_cfg,
uses_tassadar_dll,
vqmmc_enabled,
consider reordering the fields or adding explicit padding members [clang-analyzer-optin.performance.Padding]
struct sdhci_msm_host {
~~~~~~~^~~~~~~~~~~~~~~~
drivers/mmc/host/sdhci-msm.c:257:8: note: Excessive padding in 'struct sdhci_msm_host' (32 padding bytes, where 0 is optimal). Optimal fields order: pdev, core_mem, ice_mem, bus_clk, xo_clk, clk_rate, mmc, var_ops, offset, core_reset, bulk_clks, pwr_irq_wait, pwr_irq, curr_pwr_state, curr_io_level, caps_0, transfer_mode, dll_config, ddr_config, use_14lpp_dll_reset, tuning_done, calibration_done, saved_tuning_phase, use_cdclp533, pwr_irq_flag, mci_removed, restore_dll_config, use_cdr, updated_ddr_cfg, uses_tassadar_dll, vqmmc_enabled, consider reordering the fields or adding explicit padding members
struct sdhci_msm_host {
~~~~~~~^~~~~~~~~~~~~~~~
drivers/mmc/host/sdhci-msm.c:687:16: warning: Division by zero [clang-analyzer-core.DivideZero]
mclk_freq = DIV_ROUND_CLOSEST_ULL((host->clock * 8),
^
include/linux/math.h:105:2: note: expanded from macro 'DIV_ROUND_CLOSEST_ULL'
do_div(_tmp, __d); \
^~~~~~~~~~~~~~~~~
include/asm-generic/div64.h:48:26: note: expanded from macro 'do_div'
__rem = ((uint64_t)(n)) % __base; \
~~~~~~~~~~~~~~~~^~~~~~~~
drivers/mmc/host/sdhci-msm.c:627:23: note: 'xo_clk' initialized to 0
unsigned long flags, xo_clk = 0;
^~~~~~
drivers/mmc/host/sdhci-msm.c:632:6: note: Assuming field 'use_14lpp_dll_reset' is false
if (msm_host->use_14lpp_dll_reset && !IS_ERR_OR_NULL(msm_host->xo_clk))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/sdhci-msm.c:632:36: note: Left side of '&&' is false
if (msm_host->use_14lpp_dll_reset && !IS_ERR_OR_NULL(msm_host->xo_clk))
^
drivers/mmc/host/sdhci-msm.c:635:2: note: Loop condition is false. Exiting loop
spin_lock_irqsave(&host->lock, flags);
^
include/linux/spinlock.h:379:2: note: expanded from macro 'spin_lock_irqsave'
raw_spin_lock_irqsave(spinlock_check(lock), flags); \
^
include/linux/spinlock.h:240:2: note: expanded from macro 'raw_spin_lock_irqsave'
do { \
^
drivers/mmc/host/sdhci-msm.c:635:2: note: Loop condition is false. Exiting loop
spin_lock_irqsave(&host->lock, flags);
^
include/linux/spinlock.h:377:43: note: expanded from macro 'spin_lock_irqsave'
#define spin_lock_irqsave(lock, flags) \
^
drivers/mmc/host/sdhci-msm.c:642:11: note: Loop condition is false. Exiting loop
config = readl_relaxed(host->ioaddr + msm_offset->core_vendor_spec);
^
arch/riscv/include/asm/mmio.h:116:38: note: expanded from macro 'readl_relaxed'
#define readl_relaxed(c) ({ u32 __v; __io_rbr(); __v = readl_cpu(c); __io_rar(); __v; })
^
arch/riscv/include/asm/mmio.h:109:21: note: expanded from macro '__io_rbr'
#define __io_rbr() do {} while (0)
^
drivers/mmc/host/sdhci-msm.c:642:11: note: Loop condition is false. Exiting loop
config = readl_relaxed(host->ioaddr + msm_offset->core_vendor_spec);
^
arch/riscv/include/asm/mmio.h:116:70: note: expanded from macro 'readl_relaxed'
#define readl_relaxed(c) ({ u32 __v; __io_rbr(); __v = readl_cpu(c); __io_rar(); __v; })
^
arch/riscv/include/asm/mmio.h:110:21: note: expanded from macro '__io_rar'
#define __io_rar() do {} while (0)
^
drivers/mmc/host/sdhci-msm.c:644:2: note: Loop condition is false. Exiting loop
writel_relaxed(config, host->ioaddr + msm_offset->core_vendor_spec);
^
arch/riscv/include/asm/mmio.h:120:33: note: expanded from macro 'writel_relaxed'
#define writel_relaxed(v, c) ({ __io_rbw(); writel_cpu((v), (c)); __io_raw(); })
^
arch/riscv/include/asm/mmio.h:111:21: note: expanded from macro '__io_rbw'
#define __io_rbw() do {} while (0)
^
drivers/mmc/host/sdhci-msm.c:644:2: note: Loop condition is false. Exiting loop
writel_relaxed(config, host->ioaddr + msm_offset->core_vendor_spec);
vim +257 drivers/mmc/host/sdhci-msm.c
6ed4bb4387033a Vijay Viswanath 2018-06-19 256
0eb0d9f4de34a7 Georgi Djakov 2014-03-10 @257 struct sdhci_msm_host {
0eb0d9f4de34a7 Georgi Djakov 2014-03-10 258 struct platform_device *pdev;
0eb0d9f4de34a7 Georgi Djakov 2014-03-10 259 void __iomem *core_mem; /* MSM SDCC mapped address */
c93767cf64ebf4 Eric Biggers 2021-01-25 260 void __iomem *ice_mem; /* MSM ICE mapped address (if available) */
ad81d387100454 Georgi Djakov 2016-06-24 261 int pwr_irq; /* power irq */
0eb0d9f4de34a7 Georgi Djakov 2014-03-10 262 struct clk *bus_clk; /* SDHC bus voter clock */
83736352e0caaa Venkat Gopalakrishnan 2016-11-21 263 struct clk *xo_clk; /* TCXO clk needed for FLL feature of cm_dll*/
c93767cf64ebf4 Eric Biggers 2021-01-25 264 /* core, iface, cal, sleep, and ice clocks */
c93767cf64ebf4 Eric Biggers 2021-01-25 265 struct clk_bulk_data bulk_clks[5];
edc609fd19e1cd Ritesh Harjani 2016-11-21 266 unsigned long clk_rate;
0eb0d9f4de34a7 Georgi Djakov 2014-03-10 267 struct mmc_host *mmc;
83736352e0caaa Venkat Gopalakrishnan 2016-11-21 268 bool use_14lpp_dll_reset;
ff06ce417828b9 Venkat Gopalakrishnan 2016-11-21 269 bool tuning_done;
ff06ce417828b9 Venkat Gopalakrishnan 2016-11-21 270 bool calibration_done;
abf270e5c62610 Ritesh Harjani 2016-11-21 271 u8 saved_tuning_phase;
02e4293dc01362 Ritesh Harjani 2016-11-21 272 bool use_cdclp533;
c0309b3803fed9 Vijay Viswanath 2017-09-27 273 u32 curr_pwr_state;
c0309b3803fed9 Vijay Viswanath 2017-09-27 274 u32 curr_io_level;
c0309b3803fed9 Vijay Viswanath 2017-09-27 275 wait_queue_head_t pwr_irq_wait;
c0309b3803fed9 Vijay Viswanath 2017-09-27 276 bool pwr_irq_flag;
ac06fba1de8fb2 Vijay Viswanath 2018-04-20 277 u32 caps_0;
6ed4bb4387033a Vijay Viswanath 2018-06-19 278 bool mci_removed;
21f1e2d457ce67 Veerabhadrarao Badiganti 2018-11-12 279 bool restore_dll_config;
6ed4bb4387033a Vijay Viswanath 2018-06-19 280 const struct sdhci_msm_variant_ops *var_ops;
6ed4bb4387033a Vijay Viswanath 2018-06-19 281 const struct sdhci_msm_offset *offset;
a89e7bcb18081c Loic Poulain 2018-12-04 282 bool use_cdr;
a89e7bcb18081c Loic Poulain 2018-12-04 283 u32 transfer_mode;
fa56ac97922653 Veerabhadrarao Badiganti 2019-11-26 284 bool updated_ddr_cfg;
5c30f340f9e0b8 Veerabhadrarao Badiganti 2020-05-22 285 bool uses_tassadar_dll;
03591160ca192d Sarthak Garg 2020-05-22 286 u32 dll_config;
1dfbe3ff81f941 Sarthak Garg 2020-05-22 287 u32 ddr_config;
667fad5561c9eb Shaik Sajida Bhanu 2022-03-31 288 struct reset_control *core_reset;
92a2173837d2c8 Veerabhadrarao Badiganti 2020-06-23 289 bool vqmmc_enabled;
0eb0d9f4de34a7 Georgi Djakov 2014-03-10 290 };
0eb0d9f4de34a7 Georgi Djakov 2014-03-10 291
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-04-06 23:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-31 7:03 [PATCH V3] mmc: sdhci-msm: Reset GCC_SDCC_BCR register for SDHC Shaik Sajida Bhanu
2022-03-31 18:42 ` Bjorn Andersson
2022-04-06 9:46 ` Sajida Bhanu (Temp)
2022-04-06 23:31 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.