| Message ID | cover.1718241495.git.unicorn_wang@outlook.com |
|---|---|
| Headers | show |
| Series | mmc: sdhci-of-dwcmshc: enhance framework | expand |
On Thu, Jun 13, 2024 at 09:42:03AM +0800, Chen Wang wrote: > From: Chen Wang <unicorn_wang@outlook.com> > > When I tried to add a new soc to sdhci-of-dwcmshc, I found that the > existing driver code could be optimized to facilitate expansion for > the new soc. You can see another patch [sg2042-dwcmshc], which I am > working on to add SG2042 to sdhci-of-dwcmshc. > > By the way, although I believe this patch only optimizes the framework > of the code and does not change the specific logic, simple verification > is certainly better. Since I don't have rk35xx/th1520 related hardware, > it would be greatly appreciated if someone could help verify it. > > --- > > Changes in v3: > > The patch series is based on latest 'next' branch of [mmc-git]. > > Improved the dirvier code as per comments from Adrian Hunter. > Define new structure for dwcmshc platform data/ops. In addition, I organized > the code and classified the helper functions. > > Since the file changes were relatively large (though the functional logic did > not change much), I split the original patch into four for the convenience of > review. > > Changes in v2: > > Rebased on latest 'next' branch of [mmc-git]. You can simply review or test the > patches at the link [2]. > > Changes in v1: > > The patch series is based on v6.9-rc1. You can simply review or test the > patches at the link [1]. > > Link: https://lore.kernel.org/linux-mmc/cover.1713258948.git.unicorn_wang@outlook.com/ [sg2042-dwcmshc] > Link: git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git [mmc-git] > Link: https://lore.kernel.org/linux-mmc/cover.1713257181.git.unicorn_wang@outlook.com/ [1] > Link: https://lore.kernel.org/linux-mmc/cover.1714270290.git.unicorn_wang@outlook.com/ [2] > > --- > > Chen Wang (4): > mmc: sdhci-of-dwcmshc: adjust positions of helper routines > mmc: sdhci-of-dwcmshc: unify the naming of soc helper functions > mmc: sdhci-of-dwcmshc: extract init function for rk35xx/th1520 > mmc: sdhci-of-dwcmshc: add callback functions for dwcmshc > > drivers/mmc/host/sdhci-of-dwcmshc.c | 598 ++++++++++++++++------------ > 1 file changed, 339 insertions(+), 259 deletions(-) > > > base-commit: d6cd1206ffaaa890e81f5d1134856d9edd406ec6 > -- > 2.25.1 > I have tested successfully on top of 6.10-rc3 with the Lichee Pi 4a: Tested-by: Drew Fustini <dfustini@tenstorrent.com> # TH1520
On Thu, Jun 13, 2024 at 09:42:03AM +0800, Chen Wang wrote: > From: Chen Wang <unicorn_wang@outlook.com> > > When I tried to add a new soc to sdhci-of-dwcmshc, I found that the > existing driver code could be optimized to facilitate expansion for > the new soc. You can see another patch [sg2042-dwcmshc], which I am > working on to add SG2042 to sdhci-of-dwcmshc. Hi Chen, IMHO, you'd better put the sg2042 support as the last patch of this series, we want to see why the enhancement is necessary and how does it help sg2042 upstreaming. thanks > > By the way, although I believe this patch only optimizes the framework > of the code and does not change the specific logic, simple verification > is certainly better. Since I don't have rk35xx/th1520 related hardware, > it would be greatly appreciated if someone could help verify it. > > --- > > Changes in v3: > > The patch series is based on latest 'next' branch of [mmc-git]. > > Improved the dirvier code as per comments from Adrian Hunter. > Define new structure for dwcmshc platform data/ops. In addition, I organized > the code and classified the helper functions. > > Since the file changes were relatively large (though the functional logic did > not change much), I split the original patch into four for the convenience of > review. > > Changes in v2: > > Rebased on latest 'next' branch of [mmc-git]. You can simply review or test the > patches at the link [2]. > > Changes in v1: > > The patch series is based on v6.9-rc1. You can simply review or test the > patches at the link [1]. > > Link: https://lore.kernel.org/linux-mmc/cover.1713258948.git.unicorn_wang@outlook.com/ [sg2042-dwcmshc] > Link: git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git [mmc-git] > Link: https://lore.kernel.org/linux-mmc/cover.1713257181.git.unicorn_wang@outlook.com/ [1] > Link: https://lore.kernel.org/linux-mmc/cover.1714270290.git.unicorn_wang@outlook.com/ [2] > > --- > > Chen Wang (4): > mmc: sdhci-of-dwcmshc: adjust positions of helper routines > mmc: sdhci-of-dwcmshc: unify the naming of soc helper functions > mmc: sdhci-of-dwcmshc: extract init function for rk35xx/th1520 > mmc: sdhci-of-dwcmshc: add callback functions for dwcmshc > > drivers/mmc/host/sdhci-of-dwcmshc.c | 598 ++++++++++++++++------------ > 1 file changed, 339 insertions(+), 259 deletions(-) > > > base-commit: d6cd1206ffaaa890e81f5d1134856d9edd406ec6 > -- > 2.25.1 >
On 2024/6/14 18:33, Adrian Hunter wrote: > On 13/06/24 04:43, Chen Wang wrote: >> From: Chen Wang <unicorn_wang@outlook.com> [......] >> >> >> static void cv18xx_sdhci_reset(struct sdhci_host *host, u8 mask) >> { >> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> @@ -1230,46 +1270,15 @@ static int dwcmshc_probe(struct platform_device *pdev) >> host->mmc_host_ops.execute_tuning = dwcmshc_execute_tuning; >> >> if (pltfm_data == &sdhci_dwcmshc_rk35xx_pdata) { >> - rk_priv = devm_kzalloc(&pdev->dev, sizeof(struct rk35xx_priv), GFP_KERNEL); >> - if (!rk_priv) { >> - err = -ENOMEM; >> - goto err_clk; >> - } >> - >> - if (of_device_is_compatible(pdev->dev.of_node, "rockchip,rk3588-dwcmshc")) >> - rk_priv->devtype = DWCMSHC_RK3588; >> - else >> - rk_priv->devtype = DWCMSHC_RK3568; >> - >> - priv->priv = rk_priv; >> - >> - err = rk35xx_init(host, priv); >> + err = rk35xx_init(&pdev->dev, host, priv); > rk_priv is used further on, but it is not assigned anymore. You are right. It seems unnecessary to provide this patch separately just to extract the init function, and it also violates the principle of atomicity of patch modification. I will merge this patch with the next one. [......]
On 2024/6/17 22:18, Jisheng Zhang wrote: > On Thu, Jun 13, 2024 at 09:42:03AM +0800, Chen Wang wrote: >> From: Chen Wang <unicorn_wang@outlook.com> >> >> When I tried to add a new soc to sdhci-of-dwcmshc, I found that the >> existing driver code could be optimized to facilitate expansion for >> the new soc. You can see another patch [sg2042-dwcmshc], which I am >> working on to add SG2042 to sdhci-of-dwcmshc. > Hi Chen, > > IMHO, you'd better put the sg2042 support as the last patch of this > series, we want to see why the enhancement is necessary and how does > it help sg2042 upstreaming. > > thanks OK, I will consider this in next revision. Thanks, Chen >> By the way, although I believe this patch only optimizes the framework >> of the code and does not change the specific logic, simple verification >> is certainly better. Since I don't have rk35xx/th1520 related hardware, >> it would be greatly appreciated if someone could help verify it. >> >> --- >> >> Changes in v3: >> >> The patch series is based on latest 'next' branch of [mmc-git]. >> >> Improved the dirvier code as per comments from Adrian Hunter. >> Define new structure for dwcmshc platform data/ops. In addition, I organized >> the code and classified the helper functions. >> >> Since the file changes were relatively large (though the functional logic did >> not change much), I split the original patch into four for the convenience of >> review. >> >> Changes in v2: >> >> Rebased on latest 'next' branch of [mmc-git]. You can simply review or test the >> patches at the link [2]. >> >> Changes in v1: >> >> The patch series is based on v6.9-rc1. You can simply review or test the >> patches at the link [1]. >> >> Link: https://lore.kernel.org/linux-mmc/cover.1713258948.git.unicorn_wang@outlook.com/ [sg2042-dwcmshc] >> Link: git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git [mmc-git] >> Link: https://lore.kernel.org/linux-mmc/cover.1713257181.git.unicorn_wang@outlook.com/ [1] >> Link: https://lore.kernel.org/linux-mmc/cover.1714270290.git.unicorn_wang@outlook.com/ [2] >> >> --- >> >> Chen Wang (4): >> mmc: sdhci-of-dwcmshc: adjust positions of helper routines >> mmc: sdhci-of-dwcmshc: unify the naming of soc helper functions >> mmc: sdhci-of-dwcmshc: extract init function for rk35xx/th1520 >> mmc: sdhci-of-dwcmshc: add callback functions for dwcmshc >> >> drivers/mmc/host/sdhci-of-dwcmshc.c | 598 ++++++++++++++++------------ >> 1 file changed, 339 insertions(+), 259 deletions(-) >> >> >> base-commit: d6cd1206ffaaa890e81f5d1134856d9edd406ec6 >> -- >> 2.25.1 >>
On 2024/6/16 10:30, Drew Fustini wrote: > On Thu, Jun 13, 2024 at 09:42:03AM +0800, Chen Wang wrote: [......] > I have tested successfully on top of 6.10-rc3 with the Lichee Pi 4a: > > Tested-by: Drew Fustini <dfustini@tenstorrent.com> # TH1520 Thank you for your testing effort. I will continue to revise it according to other reviewers' suggestions. Regards, Chen
On 2024/6/14 18:16, Adrian Hunter wrote: > On 13/06/24 04:42, Chen Wang wrote: >> From: Chen Wang <unicorn_wang@outlook.com> >> >> This patch does not change the logic of the code, but only adjusts >> the positions of some helper functions in the file according to >> categories to facilitate future function search and maintenance. >> >> Category: helper functions (except for driver callback functions >> such as probe/remove/suspend/resume) are divided into two categories: >> >> - dwcmshc level helpers >> - soc level helpers >> >> After the adjustment, these functions will be put together according >> to category. > Please do not move any functions unless it is needed to avoid forward > declaration. > > Unnecessarily churning the code makes backports more difficult and > complicates the code history, so it should be avoided in general. Accepted. [......]
From: Chen Wang <unicorn_wang@outlook.com> When I tried to add a new soc to sdhci-of-dwcmshc, I found that the existing driver code could be optimized to facilitate expansion for the new soc. You can see another patch [sg2042-dwcmshc], which I am working on to add SG2042 to sdhci-of-dwcmshc. By the way, although I believe this patch only optimizes the framework of the code and does not change the specific logic, simple verification is certainly better. Since I don't have rk35xx/th1520 related hardware, it would be greatly appreciated if someone could help verify it. --- Changes in v3: The patch series is based on latest 'next' branch of [mmc-git]. Improved the dirvier code as per comments from Adrian Hunter. Define new structure for dwcmshc platform data/ops. In addition, I organized the code and classified the helper functions. Since the file changes were relatively large (though the functional logic did not change much), I split the original patch into four for the convenience of review. Changes in v2: Rebased on latest 'next' branch of [mmc-git]. You can simply review or test the patches at the link [2]. Changes in v1: The patch series is based on v6.9-rc1. You can simply review or test the patches at the link [1]. Link: https://lore.kernel.org/linux-mmc/cover.1713258948.git.unicorn_wang@outlook.com/ [sg2042-dwcmshc] Link: git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git [mmc-git] Link: https://lore.kernel.org/linux-mmc/cover.1713257181.git.unicorn_wang@outlook.com/ [1] Link: https://lore.kernel.org/linux-mmc/cover.1714270290.git.unicorn_wang@outlook.com/ [2] --- Chen Wang (4): mmc: sdhci-of-dwcmshc: adjust positions of helper routines mmc: sdhci-of-dwcmshc: unify the naming of soc helper functions mmc: sdhci-of-dwcmshc: extract init function for rk35xx/th1520 mmc: sdhci-of-dwcmshc: add callback functions for dwcmshc drivers/mmc/host/sdhci-of-dwcmshc.c | 598 ++++++++++++++++------------ 1 file changed, 339 insertions(+), 259 deletions(-) base-commit: d6cd1206ffaaa890e81f5d1134856d9edd406ec6