2010-11-10 00:55:39

by Minkyu Kang

[permalink] [raw]
Subject: Re: [PATCH v2] misc: adds support the FSA9480 USB Switch

Dear Greg KH,

On 12 October 2010 02:36, Greg KH <[email protected]> wrote:
> On Mon, Oct 11, 2010 at 10:32:33AM +0900, Minkyu Kang wrote:
>> The FSA9480 is a USB port accessory detector and switch.
>> This patch adds support the FSA9480 USB Switch.
>>
>> Signed-off-by: Minkyu Kang <[email protected]>
>> Signed-off-by: Kyungmin Park <[email protected]>
>> ---
>> v2:
>> ?use the threaded irq
>> ?fix the sysfs
>>
>> ?drivers/misc/Kconfig ? ?| ? ?9 +
>> ?drivers/misc/Makefile ? | ? ?1 +
>> ?drivers/misc/fsa9480.c ?| ?544 +++++++++++++++++++++++++++++++++++++++++++++++
>> ?include/linux/fsa9480.h | ? 40 ++++
>
> Why do you need a .h file here for a single driver?

Because of platform data.

>
> You are creating new sysfs files, so you are required to add a new
> Documentation/ABI file as well, please add that to this patch.

ok.

>
> And why are you exporting symbols from this driver, that should not be
> needed at all.

It's for other drivers.
For example.. When USB cable is connected, PMIC driver enable the
power for each device. (USB, uart, audio and so on..)
But if use the manual switch, PMIC driver can't know what device is enabled.
So, this function was exported.

>
>
>> +/*
>> + * fsa9480.c - FSA9480 micro USB switch device driver
>> + *
>> + * Copyright (C) 2010 Samsung Electronics
>> + * Minkyu Kang <[email protected]>
>> + * Wonguk Jeong <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>
> Are you positive about that "any later version" portion of this?

I will update the licence.

>
> thanks,
>
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

Thanks
Minkyu Kang
--
from. prom.
http://www.promsoft.net


2010-11-10 14:31:17

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2] misc: adds support the FSA9480 USB Switch

On Wed, Nov 10, 2010 at 09:55:34AM +0900, Minkyu Kang wrote:
> Dear Greg KH,

> > And why are you exporting symbols from this driver, that should not be
> > needed at all.

> It's for other drivers.
> For example.. When USB cable is connected, PMIC driver enable the
> power for each device. (USB, uart, audio and so on..)
> But if use the manual switch, PMIC driver can't know what device is enabled.
> So, this function was exported.

This seems like it would require the PMIC driver to have knowledge of
the USB switch driver which would be likely to give a system which is
rather board specific. For PMIC supply management integration the
regulator API would be more normal.

2010-11-10 17:49:14

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2] misc: adds support the FSA9480 USB Switch

On Wed, Nov 10, 2010 at 09:55:34AM +0900, Minkyu Kang wrote:
> Dear Greg KH,
>
> On 12 October 2010 02:36, Greg KH <[email protected]> wrote:
> > On Mon, Oct 11, 2010 at 10:32:33AM +0900, Minkyu Kang wrote:
> >> The FSA9480 is a USB port accessory detector and switch.
> >> This patch adds support the FSA9480 USB Switch.
> >>
> >> Signed-off-by: Minkyu Kang <[email protected]>
> >> Signed-off-by: Kyungmin Park <[email protected]>
> >> ---
> >> v2:
> >> ?use the threaded irq
> >> ?fix the sysfs
> >>
> >> ?drivers/misc/Kconfig ? ?| ? ?9 +
> >> ?drivers/misc/Makefile ? | ? ?1 +
> >> ?drivers/misc/fsa9480.c ?| ?544 +++++++++++++++++++++++++++++++++++++++++++++++
> >> ?include/linux/fsa9480.h | ? 40 ++++
> >
> > Why do you need a .h file here for a single driver?
>
> Because of platform data.

Then put it in include/linux/platform_data/ instead please.

> > You are creating new sysfs files, so you are required to add a new
> > Documentation/ABI file as well, please add that to this patch.
>
> ok.
>
> >
> > And why are you exporting symbols from this driver, that should not be
> > needed at all.
>
> It's for other drivers.
> For example.. When USB cable is connected, PMIC driver enable the
> power for each device. (USB, uart, audio and so on..)
> But if use the manual switch, PMIC driver can't know what device is enabled.
> So, this function was exported.

What drivers are using these functions? Where is the code for them?
When will they be submitted for inclusion?

thanks,

greg k-h