[PATCH] scsi: ufs: select CONFIG_NLS

[PATCH] scsi: ufs: select CONFIG_NLS

[Arnd Bergmann]

A recent change to ufshcd introduced a call to utf16s_to_utf8s,
a function that is provided by the NLS module, so we get a link
error when that is not present:

drivers/scsi/built-in.o: In function `ufshcd_read_string_desc':
:(.text+0x124d0): undefined reference to `utf16s_to_utf8s'

This adds a Kconfig 'select' statement to avoid the build error.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: b573d484e4ff ("scsi: ufs: add support to read device and string descriptors")
---
 drivers/scsi/ufs/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index 5f4530744e0a..097894a1fab5 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -37,6 +37,7 @@ config SCSI_UFSHCD
 	depends on SCSI && SCSI_DMA
 	select PM_DEVFREQ
 	select DEVFREQ_GOV_SIMPLE_ONDEMAND
+	select NLS
 	---help---
 	This selects the support for UFS devices in Linux, say Y and make
 	  sure that you know the name of your UFS host adapter (the card
-- 
2.7.0

Re: [PATCH] scsi: ufs: select CONFIG_NLS

[Martin K. Petersen]

>>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:

Arnd> A recent change to ufshcd introduced a call to utf16s_to_utf8s, a
Arnd> function that is provided by the NLS module, so we get a link
Arnd> error when that is not present:

Applied to 4.6/scsi-fixes.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: ufs: select CONFIG_NLS

[James Bottomley]

On Thu, 2016-03-17 at 13:29 +0100, Arnd Bergmann wrote:
> A recent change to ufshcd introduced a call to utf16s_to_utf8s,
> a function that is provided by the NLS module, so we get a link
> error when that is not present:
> 
> drivers/scsi/built-in.o: In function `ufshcd_read_string_desc':
> :(.text+0x124d0): undefined reference to `utf16s_to_utf8s'
> 
> This adds a Kconfig 'select' statement to avoid the build error.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: b573d484e4ff ("scsi: ufs: add support to read device and
> string descriptors")
> ---
>  drivers/scsi/ufs/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
> index 5f4530744e0a..097894a1fab5 100644
> --- a/drivers/scsi/ufs/Kconfig
> +++ b/drivers/scsi/ufs/Kconfig
> @@ -37,6 +37,7 @@ config SCSI_UFSHCD
>  	depends on SCSI && SCSI_DMA
>  	select PM_DEVFREQ
>  	select DEVFREQ_GOV_SIMPLE_ONDEMAND
> +	select NLS

This looks like a bad solution: CONFIG_NLS is nothing more than a menu
selector for the NLS subsystem.  The problem is that selecting it will
allow a kernel to be build with NLS and without NLS_DEFAULT which is
going to cause all sorts of interesting problems on boot.  I think you
really mean depends on NLS here.

James

Re: [PATCH] scsi: ufs: select CONFIG_NLS

[Arnd Bergmann]

On Tuesday 22 March 2016 07:43:19 James Bottomley wrote:
> > diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
> > index 5f4530744e0a..097894a1fab5 100644
> > --- a/drivers/scsi/ufs/Kconfig
> > +++ b/drivers/scsi/ufs/Kconfig
> > @@ -37,6 +37,7 @@ config SCSI_UFSHCD
> >       depends on SCSI && SCSI_DMA
> >       select PM_DEVFREQ
> >       select DEVFREQ_GOV_SIMPLE_ONDEMAND
> > +     select NLS
> 
> This looks like a bad solution: CONFIG_NLS is nothing more than a menu
> selector for the NLS subsystem.

It's a bit of both: CONFIG_NLS by itself controls the compilation of
the fs/nls/nls_base.c file, which has the definition of the utf16s_to_utf8s
function called by this driver. The same file also contains the
NLS subsystem with the register_nls/unregister_nls/unload_nls/load_nls
APIs that are used by the other files.

>  The problem is that selecting it will
> allow a kernel to be build with NLS and without NLS_DEFAULT which is
> going to cause all sorts of interesting problems on boot.  I think you
> really mean depends on NLS here.

I agree that 'depends on NLS' makes more sense here, however I was
just following what all other users of the API are doing. Mixing
'select' and 'depends on' can easily lead to circular dependencies,
and I was trying to avoid that.

A cleaner solution is probably to split out the utf16 access functions
from the NLS subsystem, and have all these files that currently
'select NLS' pick that other interface (or both, if necessary) instead:

drivers/acpi/device_sysfs.c
drivers/hid/hid-cp2112.c
drivers/hv/hv_fcopy.c
drivers/hv/hv_kvp.c
drivers/net/fjes/fjes_main.c
drivers/net/hyperv/rndis_filter.c
drivers/pci/pci-label.c
drivers/scsi/ufs/ufshcd.c
drivers/usb/core/message.c
drivers/usb/gadget/configfs.c
drivers/usb/gadget/u_os_desc.h
drivers/usb/gadget/usbstring.c
fs/cifs/cifs_unicode.c
fs/fat/dir.c
fs/fat/namei_vfat.c
fs/isofs/joliet.c

Not sure if that's worth the work though.

	Arnd