Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data

[Alim Akhtar]

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 linux-scsi" 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

[Alim Akhtar]

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 linux-scsi" 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

[Arnd Bergmann]

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 linux-scsi" 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

[Rob Herring]

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 linux-scsi" 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

[kbuild test robot]

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

[kbuild test robot]

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

[Arnd Bergmann]

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 linux-scsi" 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

[kbuild test robot]

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