Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
Hi Rob, On Mon, Oct 5, 2015 at 7:41 PM, Rob Herring wrote: > On Mon, Oct 5, 2015 at 4:06 AM, Arnd Bergmann wrote: >> On Monday 05 October 2015 13:44:29 Alim Akhtar wrote: >>> CCing Rob Herring, >>> >>> Hi Arnd, >>> >>> On 10/01/2015 04:59 PM, Arnd Bergmann wrote: >>> > On Thursday 01 October 2015 18:46:34 kbuild test robot wrote: >>> >> [auto build test results on v4.3-rc3 -- if it's inappropriate base, >>> >> please ignore] >>> >> >>> >> config: x86_64-allmodconfig (attached as .config) >>> >> reproduce: >>> >> git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb >>> >> # save the attached .config to linux build tree >>> >> make ARCH=x86_64 >>> >> >>> >> All error/warnings (new ones prefixed by >>): >>> >> >>> ERROR: "ufs_hba_exynos_ops" [drivers/scsi/ufs/ufshcd-pltfrm.ko] >>> undefined! >>> >> >>> >> >>> > >>> > Ah, this seems to be a case of layering violation. It would be best to >>> > restructure the code so that the exynos driver registers a platform_driver >>> > by itself for the respective DT compatible string, and then calls >>> > into the common code from its probe function, rather than having the >>> > generic driver know about the specific backends. >>> > >>> > That approach will also make the generic driver more scalable as we >>> > add further chip-specific variations, and matches what we do in other >>> > drivers. >>> > >>> >>> Looks like some discussions on ufs variant driver probe method happened >>> here [1] few months back. >>> [1]-> https://lkml.org/lkml/2015/6/3/180 >> >> Hmm, too bad we didn't catch it then, it's much more work to fix now. > > What you suggested is what is being implemented[1]. It is not merged > yet. The core is a library and the platform specific parts create the > driver. > > Rob > > [1] https://lkml.org/lkml/2015/9/2/364 Thanks for the pointer...let me have a look. At least now we have another variant to test it out. > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Regards, Alim -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
On Monday 05 October 2015 09:11:33 Rob Herring wrote: > On Mon, Oct 5, 2015 at 4:06 AM, Arnd Bergmann wrote: > > On Monday 05 October 2015 13:44:29 Alim Akhtar wrote: > >> > >> On 10/01/2015 04:59 PM, Arnd Bergmann wrote: > >> > On Thursday 01 October 2015 18:46:34 kbuild test robot wrote: > >> > Ah, this seems to be a case of layering violation. It would be best to > >> > restructure the code so that the exynos driver registers a > >> > platform_driver > >> > by itself for the respective DT compatible string, and then calls > >> > into the common code from its probe function, rather than having the > >> > generic driver know about the specific backends. > >> > > >> > That approach will also make the generic driver more scalable as we > >> > add further chip-specific variations, and matches what we do in other > >> > drivers. > >> > > >> > >> Looks like some discussions on ufs variant driver probe method happened > >> here [1] few months back. > >> [1]-> https://lkml.org/lkml/2015/6/3/180 > > > > Hmm, too bad we didn't catch it then, it's much more work to fix now. > > What you suggested is what is being implemented[1]. It is not merged > yet. The core is a library and the platform specific parts create the > driver. > > Rob > > [1] https://lkml.org/lkml/2015/9/2/364 Ah, good. Sorry for the misunderstanding on my side. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
On Mon, Oct 5, 2015 at 4:06 AM, Arnd Bergmann wrote: > On Monday 05 October 2015 13:44:29 Alim Akhtar wrote: >> CCing Rob Herring, >> >> Hi Arnd, >> >> On 10/01/2015 04:59 PM, Arnd Bergmann wrote: >> > On Thursday 01 October 2015 18:46:34 kbuild test robot wrote: >> >> [auto build test results on v4.3-rc3 -- if it's inappropriate base, >> >> please ignore] >> >> >> >> config: x86_64-allmodconfig (attached as .config) >> >> reproduce: >> >> git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb >> >> # save the attached .config to linux build tree >> >> make ARCH=x86_64 >> >> >> >> All error/warnings (new ones prefixed by >>): >> >> >> ERROR: "ufs_hba_exynos_ops" [drivers/scsi/ufs/ufshcd-pltfrm.ko] >> undefined! >> >> >> >> >> > >> > Ah, this seems to be a case of layering violation. It would be best to >> > restructure the code so that the exynos driver registers a platform_driver >> > by itself for the respective DT compatible string, and then calls >> > into the common code from its probe function, rather than having the >> > generic driver know about the specific backends. >> > >> > That approach will also make the generic driver more scalable as we >> > add further chip-specific variations, and matches what we do in other >> > drivers. >> > >> >> Looks like some discussions on ufs variant driver probe method happened >> here [1] few months back. >> [1]-> https://lkml.org/lkml/2015/6/3/180 > > Hmm, too bad we didn't catch it then, it's much more work to fix now. What you suggested is what is being implemented[1]. It is not merged yet. The core is a library and the platform specific parts create the driver. Rob [1] https://lkml.org/lkml/2015/9/2/364 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
On Monday 05 October 2015 13:44:29 Alim Akhtar wrote:
> CCing Rob Herring,
>
> Hi Arnd,
>
> On 10/01/2015 04:59 PM, Arnd Bergmann wrote:
> > On Thursday 01 October 2015 18:46:34 kbuild test robot wrote:
> >> [auto build test results on v4.3-rc3 -- if it's inappropriate base, please
> >> ignore]
> >>
> >> config: x86_64-allmodconfig (attached as .config)
> >> reproduce:
> >> git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb
> >> # save the attached .config to linux build tree
> >> make ARCH=x86_64
> >>
> >> All error/warnings (new ones prefixed by >>):
> >>
> ERROR: "ufs_hba_exynos_ops" [drivers/scsi/ufs/ufshcd-pltfrm.ko]
> undefined!
> >>
> >>
> >
> > Ah, this seems to be a case of layering violation. It would be best to
> > restructure the code so that the exynos driver registers a platform_driver
> > by itself for the respective DT compatible string, and then calls
> > into the common code from its probe function, rather than having the
> > generic driver know about the specific backends.
> >
> > That approach will also make the generic driver more scalable as we
> > add further chip-specific variations, and matches what we do in other
> > drivers.
> >
>
> Looks like some discussions on ufs variant driver probe method happened
> here [1] few months back.
> [1]-> https://lkml.org/lkml/2015/6/3/180
Hmm, too bad we didn't catch it then, it's much more work to fix now.
> And since ufshcd-pltfrm is already a platform_driver, so I just add a
> platform data for the variant driver.
> I should have add a IS_ENABLED for it to avoid the compilation error for
> other ARCH.
I still think we should do this properly here. From looking at the qcom
driver, it seems to me that the integration there was done in a way that
could not work at all:
$ git grep -w ufs_hba_qcom_vops
drivers/scsi/ufs/ufs-qcom.c: * struct ufs_hba_qcom_vops - UFS QCOM specific
variant operations
drivers/scsi/ufs/ufs-qcom.c:static const struct ufs_hba_variant_ops
ufs_hba_qcom_vops = {
drivers/scsi/ufs/ufs-qcom.c:EXPORT_SYMBOL(ufs_hba_qcom_vops);
In short, nothing references the ufs_hba_qcom_vops symbol, so the driver
is never used, and if it did, it would not work for ufs being built-in
beause the symbol is marked 'static'.
Please do the samsung front-end as I suggested and send a patch to
convert the qcom front-end the same way. No need to test that one
as the current approach doesn't work.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
CCing Rob Herring, Hi Arnd, On 10/01/2015 04:59 PM, Arnd Bergmann wrote: On Thursday 01 October 2015 18:46:34 kbuild test robot wrote: [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore] config: x86_64-allmodconfig (attached as .config) reproduce: git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): ERROR: "ufs_hba_exynos_ops" [drivers/scsi/ufs/ufshcd-pltfrm.ko] undefined! Ah, this seems to be a case of layering violation. It would be best to restructure the code so that the exynos driver registers a platform_driver by itself for the respective DT compatible string, and then calls into the common code from its probe function, rather than having the generic driver know about the specific backends. That approach will also make the generic driver more scalable as we add further chip-specific variations, and matches what we do in other drivers. Looks like some discussions on ufs variant driver probe method happened here [1] few months back. [1]-> https://lkml.org/lkml/2015/6/3/180 And since ufshcd-pltfrm is already a platform_driver, so I just add a platform data for the variant driver. I should have add a IS_ENABLED for it to avoid the compilation error for other ARCH. Thanks!! Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
On Thursday 01 October 2015 18:46:34 kbuild test robot wrote: > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please > ignore] > > config: x86_64-allmodconfig (attached as .config) > reproduce: > git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb > # save the attached .config to linux build tree > make ARCH=x86_64 > > All error/warnings (new ones prefixed by >>): > > >> ERROR: "ufs_hba_exynos_ops" [drivers/scsi/ufs/ufshcd-pltfrm.ko] undefined! > > Ah, this seems to be a case of layering violation. It would be best to restructure the code so that the exynos driver registers a platform_driver by itself for the respective DT compatible string, and then calls into the common code from its probe function, rather than having the generic driver know about the specific backends. That approach will also make the generic driver more scalable as we add further chip-specific variations, and matches what we do in other drivers. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
Hi Alim, [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore] config: x86_64-allmodconfig (attached as .config) reproduce: git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): >> ERROR: "ufs_hba_exynos_ops" [drivers/scsi/ufs/ufshcd-pltfrm.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
Hi Alim, [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore] config: i386-allmodconfig (attached as .config) reproduce: git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb # save the attached .config to linux build tree make ARCH=i386 All error/warnings (new ones prefixed by >>): >> ERROR: "ufs_hba_exynos_ops" undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
Hi Alim, [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore] config: x86_64-randconfig-s0-10011705 (attached as .config) reproduce: git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): >> drivers/built-in.o:(.rodata+0x4d9e8): undefined reference to >> `ufs_hba_exynos_ops' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
[PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data
This adds ufs_hba_exynos_ops{} to platform data, so that
exynos ufs driver can be probed.
Signed-off-by: Alim Akhtar
---
drivers/scsi/ufs/ufshcd-pltfrm.c |2 ++
drivers/scsi/ufs/ufshcd.h|1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 7db9564..39dae76 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -373,6 +373,8 @@ static int ufshcd_pltfrm_remove(struct platform_device
*pdev)
static const struct of_device_id ufs_of_match[] = {
{ .compatible = "jedec,ufs-1.1"},
+ { .compatible = "samsung,exynos7-ufs",
+.data = &ufs_hba_exynos_ops},
{},
};
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index f771cb8..776d6e0 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -687,4 +687,5 @@ static inline int ufshcd_dme_peer_get(struct ufs_hba *hba,
int ufshcd_hold(struct ufs_hba *hba, bool async);
void ufshcd_release(struct ufs_hba *hba);
+extern const struct ufs_hba_variant_ops ufs_hba_exynos_ops;
#endif /* End of Header */
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
