On 09/12/2023 17:58, Kamal Dasu wrote:
> From: Kamal Dasu <[email protected]>
>
> 74165b0 shall use a new sdio controller core version which
> requires a different reset sequence. For core reset we use
> sdhci_reset. For CMD and/or DATA reset added a new function
> to also enable SDCHI clocks SDHCI_CLOCK_CARD_EN
> SDHCI_CLOCK_INT_EN along with the SDHCI_RESET_CMD and/or
> SDHCI_RESET_DATA fields.
>
> Signed-off-by: Kamal Dasu <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Closes:
> https://lore.kernel.org/oe-kbuild-all/[email protected]/``````````````
Except malformed `````, drop all three tags.
Please test your patch on local setup. Usually many LPK reports move the
patch down the queue. :(
Best regards,
Krzysztof
On 09/12/2023 17:58, Kamal Dasu wrote:
> From: Kamal Dasu <[email protected]>
>
> With newer sdio controller core used for 74165b0 we need to update
> the compatibility with "brcm,bcm74165b0-sdhci".
>
> Signed-off-by: Kamal Dasu <[email protected]>
> ---
This wasn't tested. Using community reviewers instead of automated tools
for testing is a huge waste of our scarce resources. This makes me sad.
Best regards,
Krzysztof
On 09/12/2023 17:58, Kamal Dasu wrote:
> From: Kamal Dasu <[email protected]>
>
> With newer sdio controller core used for 74165b0 we need to update
> the compatibility with "brcm,bcm74165b0-sdhci".
Please use standard email subjects, so with the PATCH keyword in the
title. `git format-patch` helps here to create proper versioned patches.
Another useful tool is b4. Skipping the PATCH keyword makes filtering of
emails more difficult thus making the review process less convenient.
Your v2 was already made this error. I expected v3 will be correct.
Best regards,
Krzysztof
On Sat, 09 Dec 2023 11:58:15 -0500, Kamal Dasu wrote:
> From: Kamal Dasu <[email protected]>
>
> With newer sdio controller core used for 74165b0 we need to update
> the compatibility with "brcm,bcm74165b0-sdhci".
>
> Signed-off-by: Kamal Dasu <[email protected]>
> ---
> .../devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml:25:1: [error] syntax error: found character '\t' that cannot start any token (syntax)
dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.example.dts'
Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml:25:1: found a tab character that violates indentation
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml:25:1: found a tab character that violates indentation
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Hi Kamal,
kernel test robot noticed the following build warnings:
[auto build test WARNING on robh/for-next]
[also build test WARNING on soc/for-next linus/master v6.7-rc4 next-20231208]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kamal-Dasu/mmc-add-new-sdhci-reset-sequence-for-brcm-74165b0/20231210-010145
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20231209165816.39044-1-kamal.dasu%40broadcom.com
patch subject: [V3, 1/2] dt-bindings: mmc: brcm,sdhci-brcmstb: Add support for 74165b0
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231210/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml:25:1: [error] syntax error: found character '\t' that cannot start any token (syntax)
--
>> Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml:25:1: found a tab character that violates indentation
Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml: properties:honeywell,pmin-pascal: '$ref' should not be valid under {'const': '$ref'}
hint: Standard unit suffix properties don't need a type $ref
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml: properties:honeywell,pmax-pascal: '$ref' should not be valid under {'const': '$ref'}
hint: Standard unit suffix properties don't need a type $ref
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
--
>> Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml: ignoring, error parsing file
vim +25 Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 8
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 9 maintainers:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 10 - Al Cooper <[email protected]>
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 11 - Florian Fainelli <[email protected]>
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 12
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 13 properties:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 14 compatible:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 15 oneOf:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 16 - items:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 17 - enum:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 18 - brcm,bcm7216-sdhci
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 19 - const: brcm,bcm7445-sdhci
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 20 - const: brcm,sdhci-brcmstb
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 21 - items:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 22 - enum:
48e24385c58e80 Kamal Dasu 2023-12-09 23 - brcm,bcm74165b0-sdhci
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 24 - brcm,bcm7445-sdhci
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 @25 - brcm,bcm7425-sdhci
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 26 - const: brcm,sdhci-brcmstb
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 27
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 28 reg:
b16ebda6d00361 Krzysztof Kozlowski 2022-04-28 29 maxItems: 2
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 30
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 31 reg-names:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 32 items:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 33 - const: host
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 34 - const: cfg
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 35
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 36 interrupts:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 37 maxItems: 1
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 38
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 39 clocks:
2f8690ef64128b Kamal Dasu 2022-05-20 40 minItems: 1
2f8690ef64128b Kamal Dasu 2022-05-20 41 items:
2f8690ef64128b Kamal Dasu 2022-05-20 42 - description: handle to core clock for the sdhci controller
2f8690ef64128b Kamal Dasu 2022-05-20 43 - description: handle to improved 150Mhz clock for sdhci controller (Optional clock)
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 44
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 45 clock-names:
2f8690ef64128b Kamal Dasu 2022-05-20 46 minItems: 1
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 47 items:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 48 - const: sw_sdio
2f8690ef64128b Kamal Dasu 2022-05-20 49 - const: sdio_freq # Optional clock
2f8690ef64128b Kamal Dasu 2022-05-20 50
2f8690ef64128b Kamal Dasu 2022-05-20 51 clock-frequency:
2f8690ef64128b Kamal Dasu 2022-05-20 52 description:
2f8690ef64128b Kamal Dasu 2022-05-20 53 Maximum operating frequency of sdio_freq sdhci controller clock
2f8690ef64128b Kamal Dasu 2022-05-20 54 $ref: /schemas/types.yaml#/definitions/uint32
2f8690ef64128b Kamal Dasu 2022-05-20 55 minimum: 100000000
2f8690ef64128b Kamal Dasu 2022-05-20 56 maximum: 150000000
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 57
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 58 sdhci,auto-cmd12:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 59 type: boolean
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 60 description: Specifies that controller should use auto CMD12
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 61
2f8690ef64128b Kamal Dasu 2022-05-20 62 allOf:
2f8690ef64128b Kamal Dasu 2022-05-20 63 - $ref: mmc-controller.yaml#
2f8690ef64128b Kamal Dasu 2022-05-20 64 - if:
2f8690ef64128b Kamal Dasu 2022-05-20 65 properties:
2f8690ef64128b Kamal Dasu 2022-05-20 66 clock-names:
2f8690ef64128b Kamal Dasu 2022-05-20 67 contains:
2f8690ef64128b Kamal Dasu 2022-05-20 68 const: sdio_freq
2f8690ef64128b Kamal Dasu 2022-05-20 69
2f8690ef64128b Kamal Dasu 2022-05-20 70 then:
2f8690ef64128b Kamal Dasu 2022-05-20 71 required:
2f8690ef64128b Kamal Dasu 2022-05-20 72 - clock-frequency
2f8690ef64128b Kamal Dasu 2022-05-20 73
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 74 required:
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 75 - compatible
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 76 - reg
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 77 - interrupts
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 78 - clocks
2f8690ef64128b Kamal Dasu 2022-05-20 79 - clock-names
50c4ef6b8ab7d1 Florian Fainelli 2021-12-07 80
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 9/12/23 18:58, Kamal Dasu wrote:
> From: Kamal Dasu <[email protected]>
>
> 74165b0 shall use a new sdio controller core version which
> requires a different reset sequence. For core reset we use
> sdhci_reset. For CMD and/or DATA reset added a new function
> to also enable SDCHI clocks SDHCI_CLOCK_CARD_EN
SDCHI -> SDHCI
> SDHCI_CLOCK_INT_EN along with the SDHCI_RESET_CMD and/or
> SDHCI_RESET_DATA fields.
>
> Signed-off-by: Kamal Dasu <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Closes:
> https://lore.kernel.org/oe-kbuild-all/[email protected]/``````````````
???
> ---
> drivers/mmc/host/sdhci-brcmstb.c | 69 +++++++++++++++++++++++++++++---
> 1 file changed, 64 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> index c23251bb95f3..d4bd5b3c0fa4 100644
> --- a/drivers/mmc/host/sdhci-brcmstb.c
> +++ b/drivers/mmc/host/sdhci-brcmstb.c
> @@ -44,8 +44,13 @@ struct brcmstb_match_priv {
>
> static inline void enable_clock_gating(struct sdhci_host *host)
> {
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_brcmstb_priv *priv = sdhci_pltfm_priv(pltfm_host);
> u32 reg;
>
> + if (!(priv->flags & BRCMSTB_PRIV_FLAGS_GATE_CLOCK))
> + return;
> +
> reg = sdhci_readl(host, SDHCI_VENDOR);
> reg |= SDHCI_VENDOR_GATE_SDCLK_EN;
> sdhci_writel(host, reg, SDHCI_VENDOR);
> @@ -53,14 +58,54 @@ static inline void enable_clock_gating(struct sdhci_host *host)
>
> static void brcmstb_reset(struct sdhci_host *host, u8 mask)
> {
> - struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> - struct sdhci_brcmstb_priv *priv = sdhci_pltfm_priv(pltfm_host);
> -
> sdhci_and_cqhci_reset(host, mask);
>
> /* Reset will clear this, so re-enable it */
> - if (priv->flags & BRCMSTB_PRIV_FLAGS_GATE_CLOCK)
> - enable_clock_gating(host);
> + enable_clock_gating(host);
> +}
> +
> +static void brcmstb_sdhci_reset_cmd_data(struct sdhci_host *host, u8 mask)
> +{
> + ktime_t timeout;
> + u32 reg;
> + u32 new_mask = (mask & (SDHCI_RESET_CMD | SDHCI_RESET_DATA)) << 24;
> +
> + new_mask |= SDHCI_CLOCK_CARD_EN | SDHCI_CLOCK_INT_EN;
> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
Is it really necessary to write both registers together? If
so, maybe add a comment.
> + sdhci_writel(host, reg | new_mask, SDHCI_CLOCK_CONTROL);
> +
> + /* Wait max 10 ms */
> + timeout = ktime_add_ms(ktime_get(), 10);
> +
> + /* hw clears the bit when it's done */
> + while (1) {
> + bool timedout = ktime_after(ktime_get(), timeout);
> +
> + if (!(sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask))
> + break;
> + if (timedout) {
> + pr_err("%s: Reset 0x%x never completed.\n",
> + mmc_hostname(host->mmc), (int)mask);
> + sdhci_err_stats_inc(host, CTRL_TIMEOUT);
> + sdhci_dumpregs(host);
> + return;
> + }
> + udelay(10);
> + }
For new code we should try to use read_poll_timeout_atomic() or other.
> +}
> +
> +static void brcmstb_reset_74165b0(struct sdhci_host *host, u8 mask)
> +{
> + /* take care of RESET_ALL as usual */
> + if (mask & SDHCI_RESET_ALL)
> + sdhci_and_cqhci_reset(host, SDHCI_RESET_ALL);
> +
> + /* cmd and/or data treated differently on this core */
> + if (mask & (SDHCI_RESET_CMD | SDHCI_RESET_DATA))
> + brcmstb_sdhci_reset_cmd_data(host, mask);
> +
> + /* Reset will clear this, so re-enable it */
> + enable_clock_gating(host);
> }
>
> static void sdhci_brcmstb_hs400es(struct mmc_host *mmc, struct mmc_ios *ios)
> @@ -162,6 +207,13 @@ static struct sdhci_ops sdhci_brcmstb_ops_7216 = {
> .set_uhs_signaling = sdhci_brcmstb_set_uhs_signaling,
> };
>
> +static struct sdhci_ops sdhci_brcmstb_ops_74165b0 = {
> + .set_clock = sdhci_brcmstb_set_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = brcmstb_reset_74165b0,
> + .set_uhs_signaling = sdhci_brcmstb_set_uhs_signaling,
> +};
> +
> static struct brcmstb_match_priv match_priv_7425 = {
> .flags = BRCMSTB_MATCH_FLAGS_NO_64BIT |
> BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT,
> @@ -179,10 +231,17 @@ static const struct brcmstb_match_priv match_priv_7216 = {
> .ops = &sdhci_brcmstb_ops_7216,
> };
>
> +static struct brcmstb_match_priv match_priv_74165b0 = {
> + .flags = BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE,
> + .hs400es = sdhci_brcmstb_hs400es,
> + .ops = &sdhci_brcmstb_ops_74165b0,
> +};
> +
> static const struct of_device_id __maybe_unused sdhci_brcm_of_match[] = {
> { .compatible = "brcm,bcm7425-sdhci", .data = &match_priv_7425 },
> { .compatible = "brcm,bcm7445-sdhci", .data = &match_priv_7445 },
> { .compatible = "brcm,bcm7216-sdhci", .data = &match_priv_7216 },
> + { .compatible = "brcm,bcm74165b0-sdhci", .data = &match_priv_74165b0 },
> {},
> };
>