Re: [PATCH v8 1/2] mmc: ca_dw_mmc: Misc cleanup of driver

2020-06-18 Thread Alex Nemirovsky
Thanks Peng,

Could we update the state of this patch set in the patchwork DB as done? It 
still at state “New”.



> On Jun 17, 2020, at 3:14 PM, Peng Fan  wrote:
> 
> It has been applied, could you check tom's master tree?
> 
>> -Original Message-
>> From: Alex Nemirovsky [mailto:alex.nemirov...@cortina-access.com]
>> Sent: 2020年6月18日 4:25
>> To: U-Boot-Denx 
>> Cc: 李冠德 ; Peng Fan
>> ; Jaehoon Chung ; Tom Rini
>> 
>> Subject: Re: [PATCH v8 1/2] mmc: ca_dw_mmc: Misc cleanup of driver
>> 
>> Hi Peng,
>> 
>> this patch set seems to have stalled.  There was some confusion as most of
>> the driver was already pushed to master and thus a new patch was created
>> for some some misc cleanup.
>> Could you review?
>> 
>> Thanks
>> Alex
>>> On May 21, 2020, at 5:24 PM, Alex Nemirovsky
>>  wrote:
>>> 
>>> From: Arthur Li 
>>> 
>>> - Rename DT compatible name
>>> - Remove uneccessary if-statement to support 8-bit buswidth
>>> - Remove redundant error msg
>>> - Use symbolic constants in switch statement
>>> 
>>> Signed-off-by: Arthur Li 
>>> Signed-off-by: Alex Nemirovsky 
>>> CC: Peng Fan 
>>> CC: Jaehoon Chung 
>>> CC: Tom Rini 
>>> ---
>>> 
>>> Changes in v8:
>>> - No code change
>>> - Split out individual driver from Cortina Package 2 patch series to
>>> help streamline acceptence into master
>>> 
>>> Changes in v7: None
>>> Changes in v5:
>>> - Rebase code basis on v2020.04-rc5 which has  already incorporated
>>> CA eMMC initial baseline
>>> 
>>> Changes in v4:
>>> - Rename DT compatible name
>>> - Remove uneccessary if-statement to support 8-bit buswidth
>>> - Remove redundant error msg
>>> - Use symbolic constants in switch statement
>>> 
>>> drivers/mmc/ca_dw_mmc.c | 34 --
>>> 1 file changed, 12 insertions(+), 22 deletions(-)
>>> 
>>> diff --git a/drivers/mmc/ca_dw_mmc.c b/drivers/mmc/ca_dw_mmc.c index
>>> acbc850..198c41f 100644
>>> --- a/drivers/mmc/ca_dw_mmc.c
>>> +++ b/drivers/mmc/ca_dw_mmc.c
>>> @@ -19,6 +19,7 @@
>>> 
>>> #define SD_CLK_SEL_200MHZ (0x2)
>>> #define SD_CLK_SEL_100MHZ (0x1)
>>> +#define SD_CLK_SEL_50MHZ (0x0)
>>> 
>>> #define IO_DRV_SD_DS_OFFSET (16)
>>> #define IO_DRV_SD_DS_MASK   (0xff << IO_DRV_SD_DS_OFFSET)
>>> @@ -44,15 +45,11 @@ static void ca_dwmci_clksel(struct dwmci_host
>> *host)
>>> struct ca_dwmmc_priv_data *priv = host->priv;
>>> u32 val = readl(priv->sd_dll_reg);
>>> 
>>> -   if (host->bus_hz >= 2) {
>>> -   val &= ~SD_CLK_SEL_MASK;
>>> +   val &= ~SD_CLK_SEL_MASK;
>>> +   if (host->bus_hz >= 2)
>>> val |= SD_CLK_SEL_200MHZ;
>>> -   } else if (host->bus_hz >= 1) {
>>> -   val &= ~SD_CLK_SEL_MASK;
>>> +   else if (host->bus_hz >= 1)
>>> val |= SD_CLK_SEL_100MHZ;
>>> -   } else {
>>> -   val &= ~SD_CLK_SEL_MASK;
>>> -   }
>>> 
>>> writel(val, priv->sd_dll_reg);
>>> }
>>> @@ -77,14 +74,14 @@ unsigned int ca_dwmci_get_mmc_clock(struct
>> dwmci_host *host, uint freq)
>>> u8 clk_div;
>>> 
>>> switch (sd_clk_sel) {
>>> -   case 2:
>>> -   clk_div = 1;
>>> +   case SD_CLK_SEL_50MHZ:
>>> +   clk_div = 4;
>>> break;
>>> -   case 1:
>>> +   case SD_CLK_SEL_100MHZ:
>>> clk_div = 2;
>>> break;
>>> default:
>>> -   clk_div = 4;
>>> +   clk_div = 1;
>>> }
>>> 
>>> return SD_SCLK_MAX / clk_div / (host->div + 1); @@ -100,9 +97,6 @@
>>> static int ca_dwmmc_ofdata_to_platdata(struct udevice *dev)
>>> host->dev_index = 0;
>>> 
>>> host->buswidth = dev_read_u32_default(dev, "bus-width", 1);
>>> -   if (host->buswidth != 1 && host->buswidth != 4)
>>> -   return -EINVAL;
>>> -
>>> host->bus_hz = dev_read_u32_default(dev, "max-frequency",
>> 5000);
>>> priv->ds = dev_read_u32_default(dev, "io_ds", 0x33);
>>> host->fifo_mode = dev_read_bo

RE: [PATCH v8 1/2] mmc: ca_dw_mmc: Misc cleanup of driver

2020-06-17 Thread Peng Fan
It has been applied, could you check tom's master tree?

> -Original Message-
> From: Alex Nemirovsky [mailto:alex.nemirov...@cortina-access.com]
> Sent: 2020年6月18日 4:25
> To: U-Boot-Denx 
> Cc: 李冠德 ; Peng Fan
> ; Jaehoon Chung ; Tom Rini
> 
> Subject: Re: [PATCH v8 1/2] mmc: ca_dw_mmc: Misc cleanup of driver
> 
> Hi Peng,
> 
> this patch set seems to have stalled.  There was some confusion as most of
> the driver was already pushed to master and thus a new patch was created
> for some some misc cleanup.
>  Could you review?
> 
> Thanks
> Alex
> > On May 21, 2020, at 5:24 PM, Alex Nemirovsky
>  wrote:
> >
> > From: Arthur Li 
> >
> > - Rename DT compatible name
> > - Remove uneccessary if-statement to support 8-bit buswidth
> > - Remove redundant error msg
> > - Use symbolic constants in switch statement
> >
> > Signed-off-by: Arthur Li 
> > Signed-off-by: Alex Nemirovsky 
> > CC: Peng Fan 
> > CC: Jaehoon Chung 
> > CC: Tom Rini 
> > ---
> >
> > Changes in v8:
> > - No code change
> > - Split out individual driver from Cortina Package 2 patch series to
> > help streamline acceptence into master
> >
> > Changes in v7: None
> > Changes in v5:
> > - Rebase code basis on v2020.04-rc5 which has  already incorporated
> > CA eMMC initial baseline
> >
> > Changes in v4:
> > - Rename DT compatible name
> > - Remove uneccessary if-statement to support 8-bit buswidth
> > - Remove redundant error msg
> > - Use symbolic constants in switch statement
> >
> > drivers/mmc/ca_dw_mmc.c | 34 --
> > 1 file changed, 12 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/mmc/ca_dw_mmc.c b/drivers/mmc/ca_dw_mmc.c index
> > acbc850..198c41f 100644
> > --- a/drivers/mmc/ca_dw_mmc.c
> > +++ b/drivers/mmc/ca_dw_mmc.c
> > @@ -19,6 +19,7 @@
> >
> > #define SD_CLK_SEL_200MHZ (0x2)
> > #define SD_CLK_SEL_100MHZ (0x1)
> > +#define SD_CLK_SEL_50MHZ (0x0)
> >
> > #define IO_DRV_SD_DS_OFFSET (16)
> > #define IO_DRV_SD_DS_MASK   (0xff << IO_DRV_SD_DS_OFFSET)
> > @@ -44,15 +45,11 @@ static void ca_dwmci_clksel(struct dwmci_host
> *host)
> > struct ca_dwmmc_priv_data *priv = host->priv;
> > u32 val = readl(priv->sd_dll_reg);
> >
> > -   if (host->bus_hz >= 2) {
> > -   val &= ~SD_CLK_SEL_MASK;
> > +   val &= ~SD_CLK_SEL_MASK;
> > +   if (host->bus_hz >= 2)
> > val |= SD_CLK_SEL_200MHZ;
> > -   } else if (host->bus_hz >= 1) {
> > -   val &= ~SD_CLK_SEL_MASK;
> > +   else if (host->bus_hz >= 1)
> > val |= SD_CLK_SEL_100MHZ;
> > -   } else {
> > -   val &= ~SD_CLK_SEL_MASK;
> > -   }
> >
> > writel(val, priv->sd_dll_reg);
> > }
> > @@ -77,14 +74,14 @@ unsigned int ca_dwmci_get_mmc_clock(struct
> dwmci_host *host, uint freq)
> > u8 clk_div;
> >
> > switch (sd_clk_sel) {
> > -   case 2:
> > -   clk_div = 1;
> > +   case SD_CLK_SEL_50MHZ:
> > +   clk_div = 4;
> > break;
> > -   case 1:
> > +   case SD_CLK_SEL_100MHZ:
> > clk_div = 2;
> > break;
> > default:
> > -   clk_div = 4;
> > +   clk_div = 1;
> > }
> >
> > return SD_SCLK_MAX / clk_div / (host->div + 1); @@ -100,9 +97,6 @@
> > static int ca_dwmmc_ofdata_to_platdata(struct udevice *dev)
> > host->dev_index = 0;
> >
> > host->buswidth = dev_read_u32_default(dev, "bus-width", 1);
> > -   if (host->buswidth != 1 && host->buswidth != 4)
> > -   return -EINVAL;
> > -
> > host->bus_hz = dev_read_u32_default(dev, "max-frequency",
> 5000);
> > priv->ds = dev_read_u32_default(dev, "io_ds", 0x33);
> > host->fifo_mode = dev_read_bool(dev, "fifo-mode"); @@ -118,10 +112,8
> > @@ static int ca_dwmmc_ofdata_to_platdata(struct udevice *dev)
> > return -EINVAL;
> >
> > host->ioaddr = dev_read_addr_ptr(dev);
> > -   if (host->ioaddr == (void *)FDT_ADDR_T_NONE) {
> > -   printf("DWMMC: base address is invalid\n");
> > +   if (!host->ioaddr)
> > return -EINVAL;
> > -   }
> >
> > host->priv = priv;
> >
> > @@ -140,10 +132,8 @@ static int ca_dwmmc_probe(struct udevice *dev)
> > memcpy(_dwmci_dm_ops, _dwmci_ops, sizeof(struct
> dm_mmc_ops));
> >
> > dwmci_setup_cfg(>cfg, host, host->bus_hz, MIN_FREQ);
> > -   if (host->buswidth == 1) {
> > -   (>cfg)->host_caps &= ~MMC_MODE_8BIT;
> > -   (>cfg)->host_caps &= ~MMC_MODE_4BIT;
> > -   }
> > +   if (host->buswidth == 1)
> > +   (>cfg)->host_caps &= ~(MMC_MODE_8BIT |
> MMC_MODE_4BIT);
> >
> > host->mmc = >mmc;
> > host->mmc->priv = >host;
> > @@ -164,7 +154,7 @@ static int ca_dwmmc_bind(struct udevice *dev) }
> >
> > static const struct udevice_id ca_dwmmc_ids[] = {
> > -   { .compatible = "snps,dw-cortina" },
> > +   { .compatible = "cortina,ca-mmc" },
> > { }
> > };
> >
> > --
> > 2.7.4
> >



Re: [PATCH v8 1/2] mmc: ca_dw_mmc: Misc cleanup of driver

2020-06-17 Thread Alex Nemirovsky
Hi Peng,

this patch set seems to have stalled.  There was some confusion as most of the 
driver
was already pushed to master and thus a new patch was created for some some 
misc cleanup.
 Could you review?

Thanks
Alex
> On May 21, 2020, at 5:24 PM, Alex Nemirovsky 
>  wrote:
> 
> From: Arthur Li 
> 
> - Rename DT compatible name
> - Remove uneccessary if-statement to support 8-bit buswidth
> - Remove redundant error msg
> - Use symbolic constants in switch statement
> 
> Signed-off-by: Arthur Li 
> Signed-off-by: Alex Nemirovsky 
> CC: Peng Fan 
> CC: Jaehoon Chung 
> CC: Tom Rini 
> ---
> 
> Changes in v8:
> - No code change
> - Split out individual driver from Cortina Package 2 patch series
> to help streamline acceptence into master
> 
> Changes in v7: None
> Changes in v5:
> - Rebase code basis on v2020.04-rc5 which has
>  already incorporated CA eMMC initial baseline
> 
> Changes in v4:
> - Rename DT compatible name
> - Remove uneccessary if-statement to support 8-bit buswidth
> - Remove redundant error msg
> - Use symbolic constants in switch statement
> 
> drivers/mmc/ca_dw_mmc.c | 34 --
> 1 file changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mmc/ca_dw_mmc.c b/drivers/mmc/ca_dw_mmc.c
> index acbc850..198c41f 100644
> --- a/drivers/mmc/ca_dw_mmc.c
> +++ b/drivers/mmc/ca_dw_mmc.c
> @@ -19,6 +19,7 @@
> 
> #define SD_CLK_SEL_200MHZ (0x2)
> #define SD_CLK_SEL_100MHZ (0x1)
> +#define SD_CLK_SEL_50MHZ (0x0)
> 
> #define IO_DRV_SD_DS_OFFSET (16)
> #define IO_DRV_SD_DS_MASK   (0xff << IO_DRV_SD_DS_OFFSET)
> @@ -44,15 +45,11 @@ static void ca_dwmci_clksel(struct dwmci_host *host)
>   struct ca_dwmmc_priv_data *priv = host->priv;
>   u32 val = readl(priv->sd_dll_reg);
> 
> - if (host->bus_hz >= 2) {
> - val &= ~SD_CLK_SEL_MASK;
> + val &= ~SD_CLK_SEL_MASK;
> + if (host->bus_hz >= 2)
>   val |= SD_CLK_SEL_200MHZ;
> - } else if (host->bus_hz >= 1) {
> - val &= ~SD_CLK_SEL_MASK;
> + else if (host->bus_hz >= 1)
>   val |= SD_CLK_SEL_100MHZ;
> - } else {
> - val &= ~SD_CLK_SEL_MASK;
> - }
> 
>   writel(val, priv->sd_dll_reg);
> }
> @@ -77,14 +74,14 @@ unsigned int ca_dwmci_get_mmc_clock(struct dwmci_host 
> *host, uint freq)
>   u8 clk_div;
> 
>   switch (sd_clk_sel) {
> - case 2:
> - clk_div = 1;
> + case SD_CLK_SEL_50MHZ:
> + clk_div = 4;
>   break;
> - case 1:
> + case SD_CLK_SEL_100MHZ:
>   clk_div = 2;
>   break;
>   default:
> - clk_div = 4;
> + clk_div = 1;
>   }
> 
>   return SD_SCLK_MAX / clk_div / (host->div + 1);
> @@ -100,9 +97,6 @@ static int ca_dwmmc_ofdata_to_platdata(struct udevice *dev)
>   host->dev_index = 0;
> 
>   host->buswidth = dev_read_u32_default(dev, "bus-width", 1);
> - if (host->buswidth != 1 && host->buswidth != 4)
> - return -EINVAL;
> -
>   host->bus_hz = dev_read_u32_default(dev, "max-frequency", 5000);
>   priv->ds = dev_read_u32_default(dev, "io_ds", 0x33);
>   host->fifo_mode = dev_read_bool(dev, "fifo-mode");
> @@ -118,10 +112,8 @@ static int ca_dwmmc_ofdata_to_platdata(struct udevice 
> *dev)
>   return -EINVAL;
> 
>   host->ioaddr = dev_read_addr_ptr(dev);
> - if (host->ioaddr == (void *)FDT_ADDR_T_NONE) {
> - printf("DWMMC: base address is invalid\n");
> + if (!host->ioaddr)
>   return -EINVAL;
> - }
> 
>   host->priv = priv;
> 
> @@ -140,10 +132,8 @@ static int ca_dwmmc_probe(struct udevice *dev)
>   memcpy(_dwmci_dm_ops, _dwmci_ops, sizeof(struct dm_mmc_ops));
> 
>   dwmci_setup_cfg(>cfg, host, host->bus_hz, MIN_FREQ);
> - if (host->buswidth == 1) {
> - (>cfg)->host_caps &= ~MMC_MODE_8BIT;
> - (>cfg)->host_caps &= ~MMC_MODE_4BIT;
> - }
> + if (host->buswidth == 1)
> + (>cfg)->host_caps &= ~(MMC_MODE_8BIT | MMC_MODE_4BIT);
> 
>   host->mmc = >mmc;
>   host->mmc->priv = >host;
> @@ -164,7 +154,7 @@ static int ca_dwmmc_bind(struct udevice *dev)
> }
> 
> static const struct udevice_id ca_dwmmc_ids[] = {
> - { .compatible = "snps,dw-cortina" },
> + { .compatible = "cortina,ca-mmc" },
>   { }
> };
> 
> -- 
> 2.7.4
> 



[PATCH v8 1/2] mmc: ca_dw_mmc: Misc cleanup of driver

2020-05-21 Thread Alex Nemirovsky
From: Arthur Li 

- Rename DT compatible name
- Remove uneccessary if-statement to support 8-bit buswidth
- Remove redundant error msg
- Use symbolic constants in switch statement

Signed-off-by: Arthur Li 
Signed-off-by: Alex Nemirovsky 
CC: Peng Fan 
CC: Jaehoon Chung 
CC: Tom Rini 
---

Changes in v8:
- No code change
- Split out individual driver from Cortina Package 2 patch series
to help streamline acceptence into master

Changes in v7: None
Changes in v5:
- Rebase code basis on v2020.04-rc5 which has
  already incorporated CA eMMC initial baseline

Changes in v4:
- Rename DT compatible name
- Remove uneccessary if-statement to support 8-bit buswidth
- Remove redundant error msg
- Use symbolic constants in switch statement

 drivers/mmc/ca_dw_mmc.c | 34 --
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/ca_dw_mmc.c b/drivers/mmc/ca_dw_mmc.c
index acbc850..198c41f 100644
--- a/drivers/mmc/ca_dw_mmc.c
+++ b/drivers/mmc/ca_dw_mmc.c
@@ -19,6 +19,7 @@
 
 #define SD_CLK_SEL_200MHZ (0x2)
 #define SD_CLK_SEL_100MHZ (0x1)
+#define SD_CLK_SEL_50MHZ (0x0)
 
 #define IO_DRV_SD_DS_OFFSET (16)
 #define IO_DRV_SD_DS_MASK   (0xff << IO_DRV_SD_DS_OFFSET)
@@ -44,15 +45,11 @@ static void ca_dwmci_clksel(struct dwmci_host *host)
struct ca_dwmmc_priv_data *priv = host->priv;
u32 val = readl(priv->sd_dll_reg);
 
-   if (host->bus_hz >= 2) {
-   val &= ~SD_CLK_SEL_MASK;
+   val &= ~SD_CLK_SEL_MASK;
+   if (host->bus_hz >= 2)
val |= SD_CLK_SEL_200MHZ;
-   } else if (host->bus_hz >= 1) {
-   val &= ~SD_CLK_SEL_MASK;
+   else if (host->bus_hz >= 1)
val |= SD_CLK_SEL_100MHZ;
-   } else {
-   val &= ~SD_CLK_SEL_MASK;
-   }
 
writel(val, priv->sd_dll_reg);
 }
@@ -77,14 +74,14 @@ unsigned int ca_dwmci_get_mmc_clock(struct dwmci_host 
*host, uint freq)
u8 clk_div;
 
switch (sd_clk_sel) {
-   case 2:
-   clk_div = 1;
+   case SD_CLK_SEL_50MHZ:
+   clk_div = 4;
break;
-   case 1:
+   case SD_CLK_SEL_100MHZ:
clk_div = 2;
break;
default:
-   clk_div = 4;
+   clk_div = 1;
}
 
return SD_SCLK_MAX / clk_div / (host->div + 1);
@@ -100,9 +97,6 @@ static int ca_dwmmc_ofdata_to_platdata(struct udevice *dev)
host->dev_index = 0;
 
host->buswidth = dev_read_u32_default(dev, "bus-width", 1);
-   if (host->buswidth != 1 && host->buswidth != 4)
-   return -EINVAL;
-
host->bus_hz = dev_read_u32_default(dev, "max-frequency", 5000);
priv->ds = dev_read_u32_default(dev, "io_ds", 0x33);
host->fifo_mode = dev_read_bool(dev, "fifo-mode");
@@ -118,10 +112,8 @@ static int ca_dwmmc_ofdata_to_platdata(struct udevice *dev)
return -EINVAL;
 
host->ioaddr = dev_read_addr_ptr(dev);
-   if (host->ioaddr == (void *)FDT_ADDR_T_NONE) {
-   printf("DWMMC: base address is invalid\n");
+   if (!host->ioaddr)
return -EINVAL;
-   }
 
host->priv = priv;
 
@@ -140,10 +132,8 @@ static int ca_dwmmc_probe(struct udevice *dev)
memcpy(_dwmci_dm_ops, _dwmci_ops, sizeof(struct dm_mmc_ops));
 
dwmci_setup_cfg(>cfg, host, host->bus_hz, MIN_FREQ);
-   if (host->buswidth == 1) {
-   (>cfg)->host_caps &= ~MMC_MODE_8BIT;
-   (>cfg)->host_caps &= ~MMC_MODE_4BIT;
-   }
+   if (host->buswidth == 1)
+   (>cfg)->host_caps &= ~(MMC_MODE_8BIT | MMC_MODE_4BIT);
 
host->mmc = >mmc;
host->mmc->priv = >host;
@@ -164,7 +154,7 @@ static int ca_dwmmc_bind(struct udevice *dev)
 }
 
 static const struct udevice_id ca_dwmmc_ids[] = {
-   { .compatible = "snps,dw-cortina" },
+   { .compatible = "cortina,ca-mmc" },
{ }
 };
 
-- 
2.7.4