[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c1a49032-eefd-e028-5a10-845bd0762362@linaro.org>
Date: Mon, 23 Jan 2023 20:37:35 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Piyush Malgujar <pmalgujar@...vell.com>, linux-mmc@...r.kernel.org,
linux-kernel@...r.kernel.org, adrian.hunter@...el.com,
ulf.hansson@...aro.org, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, yamada.masahiro@...ionext.com,
devicetree@...r.kernel.org
Cc: jannadurai@...vell.com, cchavva@...vell.com,
Dhananjay Kangude <dkangude@...ence.com>
Subject: Re: [PATCH v2 1/5] drivers: mmc: sdhci-cadence: Reformat the code
On 23/01/2023 20:27, Piyush Malgujar wrote:
> From: Dhananjay Kangude <dkangude@...ence.com>
>
Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).
(there is no "drivers:" part)
> Reformat the code so that further SD6 changes could be
> added and it could be isolated from SD4 related code.
> Also renamed functions accordingly.
Your code makes much more than just reformat. Split trivial reformating
and renaming from code which has functional impact.
>
> Signed-off-by: Dhananjay Kangude <dkangude@...ence.com>
> Co-developed-by: Jayanthi Annadurai <jannadurai@...vell.com>
> Signed-off-by: Jayanthi Annadurai <jannadurai@...vell.com>
> Signed-off-by: Piyush Malgujar <pmalgujar@...vell.com>
> ---
> drivers/mmc/host/sdhci-cadence.c | 165 ++++++++++++++++++++-----------
> 1 file changed, 110 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 6f2de54a598773879bf339aae8450f63e1251509..cb108ff9abda32767b356bb572abdf8626746cd6 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -15,14 +15,14 @@
>
> #include "sdhci-pltfm.h"
>
> -/* HRS - Host Register Set (specific to Cadence) */
> +/* SD 4.0 Controller HRS - Host Register Set (specific to Cadence) */
> #define SDHCI_CDNS_HRS04 0x10 /* PHY access port */
> -#define SDHCI_CDNS_HRS04_ACK BIT(26)
> -#define SDHCI_CDNS_HRS04_RD BIT(25)
> -#define SDHCI_CDNS_HRS04_WR BIT(24)
> -#define SDHCI_CDNS_HRS04_RDATA GENMASK(23, 16)
> -#define SDHCI_CDNS_HRS04_WDATA GENMASK(15, 8)
> -#define SDHCI_CDNS_HRS04_ADDR GENMASK(5, 0)
> +#define SDHCI_CDNS_SD4_HRS04_ACK BIT(26)
> +#define SDHCI_CDNS_SD4_HRS04_RD BIT(25)
> +#define SDHCI_CDNS_SD4_HRS04_WR BIT(24)
> +#define SDHCI_CDNS_SD4_HRS04_RDATA GENMASK(23, 16)
> +#define SDHCI_CDNS_SD4_HRS04_WDATA GENMASK(15, 8)
> +#define SDHCI_CDNS_SD4_HRS04_ADDR GENMASK(5, 0)
>
> #define SDHCI_CDNS_HRS06 0x18 /* eMMC control */
> #define SDHCI_CDNS_HRS06_TUNE_UP BIT(15)
> @@ -38,7 +38,7 @@
> /* SRS - Slot Register Set (SDHCI-compatible) */
> #define SDHCI_CDNS_SRS_BASE 0x200
>
> -/* PHY */
> +/* PHY registers for SD4 controller */
> #define SDHCI_CDNS_PHY_DLY_SD_HS 0x00
> #define SDHCI_CDNS_PHY_DLY_SD_DEFAULT 0x01
> #define SDHCI_CDNS_PHY_DLY_UHS_SDR12 0x02
> @@ -59,24 +59,43 @@
> */
> #define SDHCI_CDNS_MAX_TUNING_LOOP 40
>
> -struct sdhci_cdns_phy_param {
> +struct sdhci_cdns_priv;
> +
> +struct sdhci_cdns_sd4_phy_param {
> u8 addr;
> u8 data;
> };
>
> +struct sdhci_cdns_data {
> + int (*phy_init)(struct sdhci_cdns_priv *priv);
> + int (*set_tune_val)(struct sdhci_host *host, unsigned int val);
> +};
> +
> +struct sdhci_cdns_sd4_phy {
> + unsigned int nr_phy_params;
> + struct sdhci_cdns_sd4_phy_param phy_params[];
> +};
Defining new structures is not a "reformat".
Best regards,
Krzysztof
Powered by blists - more mailing lists