Re: [PATCH 1/4] sysfs: Support is_visible() on binary attributes

[Greg KH]

On Mon, Sep 21, 2015 at 10:38:20AM -0300, Emilio López wrote:
> According to the sysfs header file:
> 
> "The returned value will replace static permissions defined in
>  struct attribute or struct bin_attribute."
> 
> but this isn't the case, as is_visible is only called on struct attribute
> only. This patch introduces a new is_bin_visible() function to implement
> the same functionality for binary attributes, and updates documentation
> accordingly.
> 
> Note that to keep functionality and code similar to that of normal
> attributes, the mode is now checked as well to ensure it contains only
> read/write permissions or SYSFS_PREALLOC.
> 
> Reviewed-by: Guenter Roeck 
> Signed-off-by: Emilio López 

As this should probably go through the "platform drivers" maintainer,
I'll just give you this:

Acked-by: Greg Kroah-Hartman 

So it can go through their tree and not require me to just take this
one.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] sysfs: Fix is_visible() support for binary attributes

[Greg KH]

On Wed, Sep 09, 2015 at 10:14:46AM -0300, Emilio López wrote:
> On 09/09/15 01:12, Guenter Roeck wrote:
> >On 09/08/2015 08:58 PM, Greg KH wrote:
> >>On Tue, Sep 08, 2015 at 06:10:16PM -0700, Guenter Roeck wrote:
> >>>Hi Emilio,
> >>>
> >>>On 09/08/2015 05:51 PM, Emilio López wrote:
> >>>>Hi Greg & Guenter,
> >>>>
> >>>[ ... ]
> >>>>>>>
> >>>>>>>Unless I am missing something, this is not explained anywhere,
> >>>>>>>but it is
> >>>>>>>not entirely trivial to understand. I think it should be documented.
> >>>>
> >>>>I agree. I couldn't find any mention of what this int was supposed
> >>>>to be by looking at Documentation/ (is_visible is not even mentioned
> >>>>:/) or include/linux/sysfs.h. Once we settle on something I'll
> >>>>document it before sending a v2.
> >>>>
> >>>In the include file ? No strong preference, though.
> >>>
> >>>>By the way, I wrote a quick coccinelle script to match is_visible()
> >>>>users which reference the index (included below), and it found
> >>>>references to drivers which do not seem to use any binary
> >>>>attributes, so I believe changing the index meaning shouldn't be an
> >>>>issue.
> >>>>
> >>>Good.
> >>>
> >>>>>>I agree, make i the number of the bin attribute and that should solve
> >>>>>>this issue.
> >>>>>>
> >>>>>No, that would conflict with the "normal" use of is_visible for
> >>>>>non-binary
> >>>>>attributes, and make the index all but useless, since the
> >>>>>is_visible function
> >>>>>would have to search through all the attributes anyway to figure
> >>>>>out which one
> >>>>>is being checked.
> >>>>
> >>>>Yeah, using the same indexes would be somewhat pointless, although
> >>>>not many seem to be using it anyway (only 14 files matched). Others
> >>>>seem to be comparing the attr* instead. An alternative would be to
> >>>>use negative indexes for binary attributes and positive indexes for
> >>>>normal attributes.
> >>>>
> >>>... and I probably wrote or reviewed a significant percentage of
> >>>those ;-).
> >>>
> >>>Using negative numbers for binary attributes is an interesting idea.
> >>>Kind of unusual, though. Greg, any thoughts on that ?
> >>
> >>Ick, no, that's a mess, maybe we just could drop the index alltogether?
> >>
> >
> >No, please don't. Having to manually compare dozens of index pointers
> >would be
> >even more of a mess.
> 
> So, what about keeping it the way it is in the patch, and documenting it
> thoroughly? Otherwise, we could introduce another "is_bin_visible" function
> to do this same thing but just on binary attributes, but I'd rather not add
> a new function pointer if possible.

is_bin_visiable makes sense to me instead of trying to overload the
index number in some crazy way.  There's no issue with adding another
function pointer.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] sysfs: Fix is_visible() support for binary attributes

[Greg KH]

On Tue, Sep 08, 2015 at 08:30:13AM -0700, Guenter Roeck wrote:
> Emilio,
> 
> On Tue, Sep 08, 2015 at 09:07:44AM -0300, Emilio López wrote:
> > According to the sysfs header file:
> > 
> > "The returned value will replace static permissions defined in
> >  struct attribute or struct bin_attribute."
> > 
> > but this isn't the case, as is_visible is only called on
> > struct attribute only. This patch adds the code paths required
> > to support is_visible() on binary attributes.
> > 
> > Signed-off-by: Emilio López 
> > ---
> >  fs/sysfs/group.c | 22 ++
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> > index 39a0199..eb6996a 100644
> > --- a/fs/sysfs/group.c
> > +++ b/fs/sysfs/group.c
> > @@ -37,10 +37,10 @@ static int create_files(struct kernfs_node *parent, 
> > struct kobject *kobj,
> >  {
> > struct attribute *const *attr;
> > struct bin_attribute *const *bin_attr;
> > -   int error = 0, i;
> > +   int error = 0, i = 0;
> >  
> > if (grp->attrs) {
> > -   for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++) {
> > +   for (attr = grp->attrs; *attr && !error; i++, attr++) {
> > umode_t mode = (*attr)->mode;
> >  
> > /*
> > @@ -73,13 +73,27 @@ static int create_files(struct kernfs_node *parent, 
> > struct kobject *kobj,
> > }
> >  
> > if (grp->bin_attrs) {
> > -   for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
> > +   for (bin_attr = grp->bin_attrs; *bin_attr; i++, bin_attr++) {
> > +   umode_t mode = (*bin_attr)->attr.mode;
> > +
> > if (update)
> > kernfs_remove_by_name(parent,
> > (*bin_attr)->attr.name);
> > +   if (grp->is_visible) {
> > +   mode = grp->is_visible(kobj,
> > +  &(*bin_attr)->attr, i);
> 
> With this, if 'n' is the number of non-binary attributes,
> 
> for i < n:
>   The index passed to is_visible points to a non-binary attribute.
> for i >= n:
>   The index passed to is_visible points to the (index - n)th binary
>   attribute.
> 
> Unless I am missing something, this is not explained anywhere, but it is
> not entirely trivial to understand. I think it should be documented.

I agree, make i the number of the bin attribute and that should solve
this issue.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] sysfs: Fix is_visible() support for binary attributes

[Greg KH]

On Tue, Sep 08, 2015 at 06:10:16PM -0700, Guenter Roeck wrote:
> Hi Emilio,
> 
> On 09/08/2015 05:51 PM, Emilio López wrote:
> >Hi Greg & Guenter,
> >
> [ ... ]
> 
> Unless I am missing something, this is not explained anywhere, but it is
> not entirely trivial to understand. I think it should be documented.
> >
> >I agree. I couldn't find any mention of what this int was supposed to be by 
> >looking at Documentation/ (is_visible is not even mentioned :/) or 
> >include/linux/sysfs.h. Once we settle on something I'll document it before 
> >sending a v2.
> >
> In the include file ? No strong preference, though.
> 
> >By the way, I wrote a quick coccinelle script to match is_visible() users 
> >which reference the index (included below), and it found references to 
> >drivers which do not seem to use any binary attributes, so I believe 
> >changing the index meaning shouldn't be an issue.
> >
> Good.
> 
> >>>I agree, make i the number of the bin attribute and that should solve
> >>>this issue.
> >>>
> >>No, that would conflict with the "normal" use of is_visible for non-binary
> >>attributes, and make the index all but useless, since the is_visible 
> >>function
> >>would have to search through all the attributes anyway to figure out which 
> >>one
> >>is being checked.
> >
> >Yeah, using the same indexes would be somewhat pointless, although not many 
> >seem to be using it anyway (only 14 files matched). Others seem to be 
> >comparing the attr* instead. An alternative would be to use negative indexes 
> >for binary attributes and positive indexes for normal attributes.
> >
> ... and I probably wrote or reviewed a significant percentage of those ;-).
> 
> Using negative numbers for binary attributes is an interesting idea.
> Kind of unusual, though. Greg, any thoughts on that ?

Ick, no, that's a mess, maybe we just could drop the index alltogether?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH stable-4.0] ARM: EXYNOS: Fix failed second suspend on Exynos4

[Greg KH]

On Sat, Jun 20, 2015 at 06:32:46PM +0900, Krzysztof Kozlowski wrote:
 Hi Greg,
 
 I backported the patch below for stable 4.0. It seems it was missed or
 maybe I sent it the wrong way? The backport was mailed to
 sta...@vger.kernel.org
 
 Is the patch queued for next 4.0 stable release? Should I resent it?

It's still in the to-apply queue, which is a few hundred patches long.
Don't worry, it's not lost, just in good company with other patches I'll
get to eventually.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in

Re: [PATCH RESEND] serial: samsung: Fix serial config dependencies for exynos7

[Greg KH]

On Mon, Nov 17, 2014 at 10:14:51AM +0530, Abhilash Kesavan wrote:
 From: Pankaj Dubey pankaj.du...@samsung.com
 
 Exynos7 has a similar serial controller to that present in older Samsung
 SoCs. To re-use the existing serial driver on Exynos7 we need to have
 SERIAL_SAMSUNG_UARTS_4 and SERIAL_SAMSUNG_UARTS selected. This is not
 possible because these symbols are dependent on PLAT_SAMSUNG which is
 not present for the ARMv8 based exynos7.
 
 Change the dependency of these symbols from PLAT_SAMSUNG to the serial
 driver thus making it available on exynos7. As the existing platform
 specific code making use of these symbols is related to uart driver this
 change in dependency should not cause any issues.
 
 Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com
 Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
 ---
Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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] usb: Remove references to non-existent PLAT_S5P symbol

[Greg KH]

On Tue, Nov 04, 2014 at 08:21:04PM +0100, Paul Bolle wrote:
 On Tue, 2014-11-04 at 19:33 +0100, Sylwester Nawrocki wrote:
  On 04/11/14 00:24, Greg KH wrote:
   This isn't a stable issue...
  
  Sorry for disturbing then, let me go and read the documentation again.
 
 If I remember correctly, I asked Sylwester to mark this for stable. So
 it's me that should be educated here.
 
 Why is a patch that allows the users of ARCH_S5PV210 to set
 USB_EHCI_EXYNOS or USB_OHCI_EXYNOS - which they apparently need - for a
 v3.17.y kernel, not a stable issue?

As it's something that no one seemed to ever need before (i.e. it's not
a regression fix), but it would be a new feature, I don't think it's
really a stable fix.

But feel free to convince me otherwise :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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] usb: Remove references to non-existent PLAT_S5P symbol

[Greg KH]

On Tue, Oct 07, 2014 at 11:12:07AM +0200, Sylwester Nawrocki wrote:
 The PLAT_S5P Kconfig symbol was removed in commit d78c16ccde96
 (ARM: SAMSUNG: Remove remaining legacy code). There are still
 some references left, fix that by replacing them with ARCH_S5PV210.
 
 Fixes: d78c16ccde96 (ARM: SAMSUNG: Remove remaining legacy code)
 Reported-by: Paul Bolle pebo...@tiscali.nl
 Acked-by: Jingoo Han jg1@samsung.com
 Cc: sta...@vger.kernel.org  [3.17+]

This isn't a stable issue...

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support

[Greg KH]

On Thu, Sep 25, 2014 at 10:50:22AM +0530, Vivek Gautam wrote:
 Hi Greg,
 
 
 On Mon, Sep 22, 2014 at 11:15 AM, Vivek Gautam gautam.vi...@samsung.com 
 wrote:
  Now that we have completely moved from older USB-PHY drivers
  to newer GENERIC-PHY drivers for PHYs available with USB controllers
  on Exynos series of SoCs, we can remove the support for the same
  in our host drivers too.
 
  We also defer the probe for our host in case we end up getting
  EPROBE_DEFER error when getting PHYs.
 
  Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
  Acked-by: Jingoo Han jg1@samsung.com
  Acked-by: Alan Stern st...@rowland.harvard.edu
  ---
 
 Did this one got missed for usb-next ?
 I can only see usb: host: ohci-exynos: Remove unnecessary usb-phy support
 in the next branch.
 
 Ignore me if you have already taken care of this, and plan to queue it up.

If it's not in my tree already, please resend as I don't have this in my
queue anymore.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Add support for sii9234 chip

[Greg KH]

On Fri, Apr 11, 2014 at 01:48:28PM +0200, Tomasz Stanislawski wrote:
 Hi everyone,
 This patchset adds support for sii9234 HD Mobile Link Bridge.  The chip is 
 used
 to convert HDMI signal into MHL.  The driver enables HDMI output on Trats and
 Trats2 boards.
 
 The code is based on the driver [1] developed by:
Adam Hampson ahamp...@sta.samsung.com
Erik Gilling konk...@android.com
 with additional contributions from:
Shankar Bandal shanka...@samsung.com
Dharam Kumar dharam...@samsung.com
 
 The drivers architecture was greatly simplified and transformed into a form
 accepted (hopefully) by opensource community.  The main differences from
 original code are:
 * using single I2C client instead of 4 subclients
 * remove all logic non-related to establishing HDMI link
 * simplify error handling
 * rewrite state machine in interrupt handler
 * wakeup and discovery triggered by an extcon event
 * integrate with Device Tree
 
 For now, the driver is added to drivers/misc/ directory because it has neigher
 userspace nor kernel interface.  The chip is capable of receiving and
 processing CEC events, so the driver may export an input device in /dev/ in 
 the
 future.  However CEC could be also handled by HDMI driver.
 
 I kindly ask for suggestions about the best location for this driver.

It really is an extcon driver, so why not put it in drivers/extcon?  And
that might solve any build issues you have if you don't select extcon in
your .config file and try to build this code :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] misc: add sii9234 driver

[Greg KH]

On Fri, Apr 11, 2014 at 01:48:29PM +0200, Tomasz Stanislawski wrote:
 diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
 index 1cb7408..3b7f266 100644
 --- a/drivers/misc/Kconfig
 +++ b/drivers/misc/Kconfig
 @@ -515,6 +515,14 @@ config SRAM
 the genalloc API. It is supposed to be used for small on-chip SRAM
 areas found on many SoCs.
  
 +config SII9234
 + tristate Silicon Image SII9234 Driver
 + depends on I2C

I doubt this is the only dependency, right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] serial: pl011: Move uart_register_driver call to device probe

[Greg KH]

On Mon, Jan 20, 2014 at 10:04:15AM +, Russell King - ARM Linux wrote:
 On Mon, Jan 20, 2014 at 02:32:35PM +0530, Tushar Behera wrote:
  uart_register_driver call binds the driver to a specific device
  node through tty_register_driver call. This should typically happen
  during device probe call.
  
  In a multiplatform scenario, it is possible that multiple serial
  drivers are part of the kernel. Currently the driver registration fails
  if multiple serial drivers with same default major/minor numbers are
  included in the kernel.
  
  A typical case is observed with amba-pl011 and samsung-uart drivers.
 
 NAK.  There should not be any other driver using amba-pl011's device numbers.

I agree, but there is.  And because of that, moving the registration to
the probe call fixes the issue with building a kernel with all of the
drivers built into them, so I'm going to take both of these patches, as
it does solve that problem, while still allowing the device number
collision to happen.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] serial: pl011: Move uart_register_driver call to device probe

[Greg KH]

On Thu, Feb 13, 2014 at 06:15:59PM +, Russell King - ARM Linux wrote:
 On Thu, Feb 13, 2014 at 10:12:16AM -0800, Greg KH wrote:
  On Mon, Jan 20, 2014 at 10:04:15AM +, Russell King - ARM Linux wrote:
   On Mon, Jan 20, 2014 at 02:32:35PM +0530, Tushar Behera wrote:
uart_register_driver call binds the driver to a specific device
node through tty_register_driver call. This should typically happen
during device probe call.

In a multiplatform scenario, it is possible that multiple serial
drivers are part of the kernel. Currently the driver registration fails
if multiple serial drivers with same default major/minor numbers are
included in the kernel.

A typical case is observed with amba-pl011 and samsung-uart drivers.
   
   NAK.  There should not be any other driver using amba-pl011's device 
   numbers.
  
  I agree, but there is.  And because of that, moving the registration to
  the probe call fixes the issue with building a kernel with all of the
  drivers built into them, so I'm going to take both of these patches, as
  it does solve that problem, while still allowing the device number
  collision to happen.
 
 So what happens when two _devices_ are probed by this driver at the same
 time?

The bus that the driver is on will not allow that to happen, I thought
we went through this before...

And yes, devices on different busses could cause problems, but is that
the case here for these devices?  At first glance, I don't think that
can happen for these drivers.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] serial: pl011: Move uart_register_driver call to device probe

[Greg KH]

On Thu, Feb 13, 2014 at 06:42:49PM +, Russell King - ARM Linux wrote:
 On Thu, Feb 13, 2014 at 10:27:01AM -0800, Greg KH wrote:
  On Thu, Feb 13, 2014 at 06:15:59PM +, Russell King - ARM Linux wrote:
   On Thu, Feb 13, 2014 at 10:12:16AM -0800, Greg KH wrote:
On Mon, Jan 20, 2014 at 10:04:15AM +, Russell King - ARM Linux 
wrote:
 On Mon, Jan 20, 2014 at 02:32:35PM +0530, Tushar Behera wrote:
  uart_register_driver call binds the driver to a specific device
  node through tty_register_driver call. This should typically happen
  during device probe call.
  
  In a multiplatform scenario, it is possible that multiple serial
  drivers are part of the kernel. Currently the driver registration 
  fails
  if multiple serial drivers with same default major/minor numbers are
  included in the kernel.
  
  A typical case is observed with amba-pl011 and samsung-uart drivers.
 
 NAK.  There should not be any other driver using amba-pl011's device 
 numbers.

I agree, but there is.  And because of that, moving the registration to
the probe call fixes the issue with building a kernel with all of the
drivers built into them, so I'm going to take both of these patches, as
it does solve that problem, while still allowing the device number
collision to happen.
   
   So what happens when two _devices_ are probed by this driver at the same
   time?
  
  The bus that the driver is on will not allow that to happen, I thought
  we went through this before...
  
  And yes, devices on different busses could cause problems, but is that
  the case here for these devices?  At first glance, I don't think that
  can happen for these drivers.
 
 We went through this before, and I stated the paths, and no one disagreed
 with that.
 
 It /is/ racy.

Ok, I just went and looked at the uart driver register path, and I don't
see the race (note, if there is one, it's there today, regardless of
this patch).

uart_register_driver() fils in a bunch of fields, and passes control off
to tty_register_driver().  If the minor/major number allocation here
fails, due to collisions, then an error occurs, and things back out
safely.

If the allocation succeeds, then we lock the list of drivers, add them
to the list, then register the tty device with the subsystem for how
ever many tty devices were asked for.  Yes, the locking might be wrong
here, in the tty layer, but again, that's nothing new created by this
patch.

So I fail to see the problem with applying this patch, as it solves a
problem people are having, and should be fine given that no hardware
with both of these devices will ever be present at the same time.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] serial: pl011: Move uart_register_driver call to device probe

[Greg KH]

On Fri, Feb 14, 2014 at 12:07:17AM +, Russell King - ARM Linux wrote:
 On Thu, Feb 13, 2014 at 03:26:06PM -0800, Greg KH wrote:
  On Thu, Feb 13, 2014 at 06:42:49PM +, Russell King - ARM Linux wrote:
   We went through this before, and I stated the paths, and no one disagreed
   with that.
   
   It /is/ racy.
  
  Ok, I just went and looked at the uart driver register path, and I don't
  see the race (note, if there is one, it's there today, regardless of
  this patch).
 
 The race isn't the uart code, it's the driver model.
 
 Consider what happens when this happens:
 
 * Two pl011 devices get registered at the same time by two different
   threads.

How?  What two different busses will see this same device?  The amba bus
code should prevent that from happening, right?  If not, there's bigger
problems in that bus code :)

That's where this problem should be fixed, if there is one, otherwise
this same issue would be there for any type of driver that calles into
the uart core, right?

 * Both devices have a lock taken on the _device_ itself before matching
   against the driver.
 
 * Both devices get matched to the same driver.
 
 * Both devices are passed into the driver's probe function.
 
 * Both check uart_reg.state, both call uart_register_driver() on that
   at the same time, which results in two allocations inside
   uart_register_driver(), one gets overwritten...
 
 So, the /only/ thing which stops this happening is that the devices
 are generally available before the driver is registered, and driver
 registration results in devices being probed serially.  Moreover, both
 attempt to call tty_register_driver()... one succeeds, the other fails.
 
 However, what about the userspace bind/unbind methods.  Yes, userspace
 can ask the driver core to unbind devices from a driver or bind - and
 again, there's no per-driver locking here.  So, if you can trigger two
 concurrent binds from userspace, you hit the same race as above.
 
 So, if you want to accept these patches, go ahead, introduce races, but
 personally I'd recommend plugging these races.

The only way to solve this would be to do it in the bus, I don't see
anything here that makes it any racier than it currently is.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] serial: samsung: Move uart_register_driver call to device probe

[Greg KH]

On Mon, Jan 20, 2014 at 10:05:30AM +, Russell King - ARM Linux wrote:
 On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote:
  uart_register_driver call binds the driver to a specific device
  node through tty_register_driver call. This should typically happen
  during device probe call.
  
  In a multiplatform scenario, it is possible that multiple serial
  drivers are part of the kernel. Currently the driver registration fails
  if multiple serial drivers with same default major/minor numbers are
  included in the kernel.
  
  A typical case is observed with amba-pl011 and samsung-uart drivers.
 
 The samsung-uart driver is at fault here - the major/minor numbers were
 officially registered to amba-pl011.  Samsung needs to be fixed properly.

I agree, the Samsung driver is broken here, but that's no reason why
these two drivers can't register with the tty layer _after_ the hardware
is detected, and not before.

That saves resources on systems that build the drivers in, yet do not
have the hardware present, which is always a good thing.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] serial: samsung: Move uart_register_driver call to device probe

[Greg KH]

On Mon, Jan 20, 2014 at 09:32:06PM +, Russell King - ARM Linux wrote:
 On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote:
  On Mon, Jan 20, 2014 at 10:05:30AM +, Russell King - ARM Linux wrote:
   On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote:
uart_register_driver call binds the driver to a specific device
node through tty_register_driver call. This should typically happen
during device probe call.

In a multiplatform scenario, it is possible that multiple serial
drivers are part of the kernel. Currently the driver registration fails
if multiple serial drivers with same default major/minor numbers are
included in the kernel.

A typical case is observed with amba-pl011 and samsung-uart drivers.
   
   The samsung-uart driver is at fault here - the major/minor numbers were
   officially registered to amba-pl011.  Samsung needs to be fixed properly.
  
  I agree, the Samsung driver is broken here, but that's no reason why
  these two drivers can't register with the tty layer _after_ the hardware
  is detected, and not before.
  
  That saves resources on systems that build the drivers in, yet do not
  have the hardware present, which is always a good thing.
 
 Great, so what you're saying is that we need to wait until the first
 device calls into the probe function.  What about removal... how does
 a driver know when it's last device has been removed to de-register
 that?

The bus that the device is on handles that, right?

 I guess it needs the driver model to provide some way to know when a
 driver is completely unbound - but isn't that racy?

How is it racy?  That's how the driver model works...

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] serial: samsung: Move uart_register_driver call to device probe

[Greg KH]

On Mon, Jan 20, 2014 at 11:16:03PM +, Russell King - ARM Linux wrote:
 On Mon, Jan 20, 2014 at 03:11:41PM -0800, Greg KH wrote:
  On Mon, Jan 20, 2014 at 09:32:06PM +, Russell King - ARM Linux wrote:
   On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote:
On Mon, Jan 20, 2014 at 10:05:30AM +, Russell King - ARM Linux 
wrote:
 On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote:
  uart_register_driver call binds the driver to a specific device
  node through tty_register_driver call. This should typically happen
  during device probe call.
  
  In a multiplatform scenario, it is possible that multiple serial
  drivers are part of the kernel. Currently the driver registration 
  fails
  if multiple serial drivers with same default major/minor numbers are
  included in the kernel.
  
  A typical case is observed with amba-pl011 and samsung-uart drivers.
 
 The samsung-uart driver is at fault here - the major/minor numbers 
 were
 officially registered to amba-pl011.  Samsung needs to be fixed 
 properly.

I agree, the Samsung driver is broken here, but that's no reason why
these two drivers can't register with the tty layer _after_ the hardware
is detected, and not before.

That saves resources on systems that build the drivers in, yet do not
have the hardware present, which is always a good thing.
   
   Great, so what you're saying is that we need to wait until the first
   device calls into the probe function.  What about removal... how does
   a driver know when it's last device has been removed to de-register
   that?
  
  The bus that the device is on handles that, right?
  
   I guess it needs the driver model to provide some way to know when a
   driver is completely unbound - but isn't that racy?
  
  How is it racy?  That's how the driver model works...
 
 Think about what happens when the last device unregisters, but a new
 device comes along and is probed.
 
 I don't believe the driver model has any locking to prevent a drivers
 -probe function running concurrently with it's -remove function for
 two (or more) devices.

The bus prevents this from happening.

 The locking against this is done on a per-device basis, not a per-driver
 basis.

No, on a per-bus basis.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] serial: samsung: Move uart_register_driver call to device probe

[Greg KH]

On Tue, Jan 21, 2014 at 12:07:06AM +, Russell King - ARM Linux wrote:
 On Mon, Jan 20, 2014 at 03:51:28PM -0800, Greg KH wrote:
  On Mon, Jan 20, 2014 at 11:16:03PM +, Russell King - ARM Linux wrote:
   I don't believe the driver model has any locking to prevent a drivers
   -probe function running concurrently with it's -remove function for
   two (or more) devices.
  
  The bus prevents this from happening.
  
   The locking against this is done on a per-device basis, not a per-driver
   basis.
  
  No, on a per-bus basis.
 
 I don't see it.
 
 Let's start from driver_register().

Which happens from module probing, which is single-threaded, right?

Or from module_init callbacks, which is single-threaded.

Normally, busses never add devices (which is what drivers bind to),
except in a single-at-a-time fashion, unless they really know what they
are doing (i.e. PCI had multi-threaded device probing for a while, don't
remember if it still does...)


 This takes no locks and calls bus_add_driver().
 This also takes no locks and calls driver_attach().
 This walks the list of devices calling __driver_attach() for each.
 __driver_attach() tries to match the device against the driver,
 locks the parent device if one exists, and the device which is about
 to be probed.  It then calls driver_probe_device().
 driver_probe_device() inserts a runtime barrier and calls really_probe().
 really_probe() ultimately calls either the bus -probe method or the
 driver -probe method.
 
 At no point in that sequence do I see anything which does any locking
 on a per-driver basis.  Let's look at device_add().
 
 device_add() calls bus_probe_device(), which then calls device_attach().
 device_attach() takes the device's lock, and walks the list of drivers
 calling __device_attach() on each driver.  This then calls down into
 driver_probe_device(), and the path is the same as the above.
 
 I don't see any per-driver locking here either.
 
 I've checked the klist stuff, don't see anything there.  Ditto for
 bus_for_each_drv().
 
 If you think there's a per-driver lock that's held over probes or removes,
 please point it out.  I'm fairly certain that there isn't, because we have
 to be able to deal with recursive probes (yes, we've had to deal with
 those in the past.)

Hm, you are right, I think that's why we had to remove the locks.  The
klist stuff handles us getting the needed locks for managing our
internal lists of devices and drivers, and those should be fine.

So, let's go back to your original worry, what are you concerned about?
A device being removed while probe() is called?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] tty: Fallback to use dynamic major number

[Greg KH]

On Thu, Jan 16, 2014 at 10:33:22AM +0530, Tushar Behera wrote:
 In a multi-platform scenario, the hard-coded major/minor numbers in
 serial drivers may conflict with each other. A typical scenario is
 observed with amba-pl011 and samsung-uart drivers, both of these
 drivers use same set of major/minor numbers. If both of these drivers
 are enabled, probe of samsung-uart driver fails because the desired
 node is busy.

Why would both drivers ever be loaded at the same time?  Don't they bind
to different hardware devices, and as such, never will be in the same
system?

The driver shouldn't be registering it's tty devices if the hardware
isn't present in the system, so how can this codepath ever happen?

 The issue is fixed by adding a fallback in driver core, so that we can
 use dynamic major number in case device node allocation fails for
 hard-coded major/minor number.

Did you test this out?  You still get userspace breakage with it :(

 Signed-off-by: Tushar Behera tushar.beh...@linaro.org
 ---
 Hi Greg,
 
 This is my second attempt at getting this fixed. Let me know if you are
 okay with this.
 
 Initial approach to fix this problem was by forcing samsung-uart driver
 to use dynamic major number, but it was rejected [1] because of possible
 user-space breakage.

possible?  Heh, no, that should be real.

Sorry, but I think you are trying to fix a problem that never actually
happens in real life, or in valid configurations...

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] tty: Fallback to use dynamic major number

[Greg KH]

On Thu, Jan 16, 2014 at 05:08:14PM +, Mark Brown wrote:
 On Thu, Jan 16, 2014 at 08:18:41AM -0800, Greg KH wrote:
  On Thu, Jan 16, 2014 at 10:33:22AM +0530, Tushar Behera wrote:
   In a multi-platform scenario, the hard-coded major/minor numbers in
   serial drivers may conflict with each other. A typical scenario is
   observed with amba-pl011 and samsung-uart drivers, both of these
   drivers use same set of major/minor numbers. If both of these drivers
   are enabled, probe of samsung-uart driver fails because the desired
   node is busy.
 
  Why would both drivers ever be loaded at the same time?  Don't they bind
  to different hardware devices, and as such, never will be in the same
  system?
 
 No, the issue is happening because the drivers are registering things
 at module load time and not at device instantiation time.

That's a driver bug that could easily be fixed, instead of hacking up
the tty core :)

 While the drivers won't actually be run together things like the
 multiplatform defconfig and distro configs will build them in since
 people tend to like to get serial early on.

Build them into the kernel and not as modules?

  The driver shouldn't be registering it's tty devices if the hardware
  isn't present in the system, so how can this codepath ever happen?
 
 A quick and unscientific poll of serial drivers seems to suggest that
 uart_register_driver() is normally called when the module is loaded (if
 it shouldn't be this is at least a very common error pattern).

It's a common error pattern :)

   The issue is fixed by adding a fallback in driver core, so that we can
   use dynamic major number in case device node allocation fails for
   hard-coded major/minor number.
 
  Did you test this out?  You still get userspace breakage with it :(
 
 Yes.  I should put my hands up and say that this was my idea.  The
 theory is that any system using static allocation for a device shouldn't
 be affected (modulo races, it's not perfect obviously), any currently
 broken system with a userspace with a dynamic dev will start working and
 any breakage is confined to drivers which duplicated major numbers (and
 even then only on systems with static /dev).
 
 The other solutions I can think of are moving tty_register_driver() to
 device probe time, allowing tty_register_driver() to accept duplicate
 majors and the complaining when the devices actually get instantiated
 or changing major numbers where there is a duplicate which is guaranteed
 to break at least some userspaces with static devices (which was the
 original patch you complained about).  The first two solutions look a
 bit fun though perhaps they fall out more easily than I suspect.

How about fixing the drivers as I mentioned above, to not register with
the uart or tty core until they know that their hardware is actually
present in the system?  That would keep any tty core hacks from being
needed.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/2] dwc2/s3c-hsotg: Initial steps to combine the 2 driver

[Greg KH]

On Tue, Jan 14, 2014 at 05:01:00AM -0600, dingu...@altera.com wrote:
 From: Dinh Nguyen dingu...@altera.com
 
 Hi,
 
 I'm starting work downstream on combining the DWC2 host driver and the 
 s3c-hsotg
 gadget driver into a dual-role OTG driver. Before I go further, I was hoping 
 to
 solicit comments on whether or not my initial approach is correct? I know 
 there
 are plans to combine the 2, so would like to solicit comments/suggestions so
 that I can also upstream it as well.
 
 These 2 patches:
 
 * Moves the DWC2 driver out of drivers/staging into drivers/usb/dwc2/

This already happened yesterday in my tree, so you should see this in
linux-next by now, no need to do it again :)

 * Moves the s3c-hsotg driver into drivers/usb/dwc2/
 * Delete s3c-hsotg.h 
 * Make the s3c-hsotg.c file use the defines in hw.h from the DWC2 driver.
 
 This initial patch has been tested on the SOCFPGA platform only in Host-only
 and Gadget-only mode.
 
 The next step would be to do the combining of the driver into a dual-role OTG
 driver.

I was told that merging the two of these isn't going to work as the
silicon is just too different, which is why I allowed the code to move
out of staging.  If you feel differently, and think you can combine the
two drivers, that's wonderful, I'll gladly take patches to do so, but be
sure to test on the proper platforms to make sure nothing breaks.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/2] dwc2/s3c-hsotg: Initial steps to combine the 2 driver

[Greg KH]

On Tue, Jan 14, 2014 at 08:57:12PM +, Paul Zimmerman wrote:
  From: Dinh Nguyen [mailto:dingu...@altera.com]
  Sent: Tuesday, January 14, 2014 12:46 PM
  
  On Tue, 2014-01-14 at 06:21 -0800, Greg KH wrote:
   On Tue, Jan 14, 2014 at 05:01:00AM -0600, dingu...@altera.com wrote:
From: Dinh Nguyen dingu...@altera.com
   
Hi,
   
I'm starting work downstream on combining the DWC2 host driver and the 
s3c-hsotg
gadget driver into a dual-role OTG driver. Before I go further, I was 
hoping to
solicit comments on whether or not my initial approach is correct? I 
know there
are plans to combine the 2, so would like to solicit 
comments/suggestions so
that I can also upstream it as well.
   
These 2 patches:
   
* Moves the DWC2 driver out of drivers/staging into drivers/usb/dwc2/
  
   This already happened yesterday in my tree, so you should see this in
   linux-next by now, no need to do it again :)
  
  
  I see it now. Thanks for the pointer.
  
* Moves the s3c-hsotg driver into drivers/usb/dwc2/
* Delete s3c-hsotg.h
* Make the s3c-hsotg.c file use the defines in hw.h from the DWC2 
driver.
   
This initial patch has been tested on the SOCFPGA platform only in 
Host-only
and Gadget-only mode.
   
The next step would be to do the combining of the driver into a 
dual-role OTG
driver.
  
   I was told that merging the two of these isn't going to work as the
   silicon is just too different, which is why I allowed the code to move
   out of staging.  If you feel differently, and think you can combine the
   two drivers, that's wonderful, I'll gladly take patches to do so, but be
   sure to test on the proper platforms to make sure nothing breaks.
  
  
  I wasn't aware of the silicon differences. I just took the s3c-hsotg
  driver as is and it worked fine on my version 2.93a of the USB IP. I'll
  search the ML for information, or perhaps Paul can comment?
 
 I think Greg is thinking of the octeon-usb driver in staging [1], not
 the s3c-hsotg driver. The plan was always to eventually merge dwc2 with
 s3c-hsotg.

Yes, I'm totally confused, you are right.

Nevermind then, Dinh, if you want to redo your patch after 3.14-rc1 is
out, that would be great as merging the drivers together can be done
easier after that development point.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Greg KH]

On Fri, Dec 27, 2013 at 03:47:31PM +0530, Tushar Behera wrote:
 On 27 December 2013 12:08, Greg KH gre...@linuxfoundation.org wrote:
  On Fri, Dec 27, 2013 at 12:00:20PM +0530, Tushar Behera wrote:
  On 27 December 2013 10:48, Greg KH gre...@linuxfoundation.org wrote:
   On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:
 
 [ ... ]
 
   @@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
 .nr = CONFIG_SERIAL_SAMSUNG_UARTS,
 .cons   = S3C24XX_SERIAL_CONSOLE,
 .dev_name   = S3C24XX_SERIAL_NAME,
   - .major  = S3C24XX_SERIAL_MAJOR,
   - .minor  = S3C24XX_SERIAL_MINOR,
  
   Doesn't this break existing systems and configurations that are
   expecting 204:64 as the location of this serial port?
  
 
  I tested this on Exynos4210-Origen, Exynos5250-Arndale board, it works
  fine there. I haven't tested on any older boards.
 
  How did it work?  You are relying on some userspace tools to do this
  properly, right?  What about systems without those specific tools?
 
 
 Enabling CONFIG_DEVTMPFS, all the /dev/ttySACn nodes are generated
 and the appropriate console is specified through command line
 argument.

But what about systems that rely on a hard-coded /dev?

Look, I'm all for making everyone use devtmpfs, but just changing
major:minor numbers for drivers isn't ok, as you are changing the
userspace ABI for the device.

Please realize what you are asking for here, I really don't think you
grasp it given that you didn't ask any of the maintainers of this driver
about the change in the first place.

Please get approval for this patch from others within Linaro before
sending it out again.  Linaro has a process in place for this type of
thing, please use it, otherwise it makes people like me really grumpy
and upset and causes me to yell at people at their conferences.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Greg KH]

On Fri, Dec 27, 2013 at 12:00:20PM +0530, Tushar Behera wrote:
 On 27 December 2013 10:48, Greg KH gre...@linuxfoundation.org wrote:
  On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:
  The hard-coded values clash with the values set for amba-pl011 serial
  driver. Because of this there is no serial output on Samsung boards
  if amba-pl011 is enabled alongwith samsung-serial driver.
 
  Remove the hardcoded values and let the framework decide on
  appropriate major/minor number. This is required for multi-platform
  development work on Exynos platform.
 
  Signed-off-by: Tushar Behera tushar.beh...@linaro.org
  ---
   drivers/tty/serial/samsung.c |4 
   1 file changed, 4 deletions(-)
 
  diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
  index c1af04d..9c20543 100644
  --- a/drivers/tty/serial/samsung.c
  +++ b/drivers/tty/serial/samsung.c
  @@ -56,8 +56,6 @@
   /* UART name and device definitions */
 
   #define S3C24XX_SERIAL_NAME  ttySAC
  -#define S3C24XX_SERIAL_MAJOR 204
  -#define S3C24XX_SERIAL_MINOR 64
 
   /* macros to change one thing to another */
 
  @@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
.nr = CONFIG_SERIAL_SAMSUNG_UARTS,
.cons   = S3C24XX_SERIAL_CONSOLE,
.dev_name   = S3C24XX_SERIAL_NAME,
  - .major  = S3C24XX_SERIAL_MAJOR,
  - .minor  = S3C24XX_SERIAL_MINOR,
 
  Doesn't this break existing systems and configurations that are
  expecting 204:64 as the location of this serial port?
 
 
 I tested this on Exynos4210-Origen, Exynos5250-Arndale board, it works
 fine there. I haven't tested on any older boards.

How did it work?  You are relying on some userspace tools to do this
properly, right?  What about systems without those specific tools?

  Why change this one and not the amba-pl011 driver?
 
 
 I could only test this driver, so thought of changing this rather than
 modifying amba-pl011 driver. I don't have any other reason.

Please get the samsung driver maintainer to agree with this and sign off
on it before trying to get it merged again.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Greg KH]

On Fri, Dec 27, 2013 at 10:43:23AM +0400, Alexander Shiyan wrote:
 Hello.
  On Fri, Dec 27, 2013 at 12:00:20PM +0530, Tushar Behera wrote:
   On 27 December 2013 10:48, Greg KH gre...@linuxfoundation.org wrote:
On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:
The hard-coded values clash with the values set for amba-pl011 serial
driver. Because of this there is no serial output on Samsung boards
if amba-pl011 is enabled alongwith samsung-serial driver.
   
Remove the hardcoded values and let the framework decide on
appropriate major/minor number. This is required for multi-platform
development work on Exynos platform.
   
Signed-off-by: Tushar Behera tushar.beh...@linaro.org
 ...
 #define S3C24XX_SERIAL_NAME  ttySAC
-#define S3C24XX_SERIAL_MAJOR 204
-#define S3C24XX_SERIAL_MINOR 64
   
 /* macros to change one thing to another */
   
@@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
  .nr = CONFIG_SERIAL_SAMSUNG_UARTS,
  .cons   = S3C24XX_SERIAL_CONSOLE,
  .dev_name   = S3C24XX_SERIAL_NAME,
- .major  = S3C24XX_SERIAL_MAJOR,
- .minor  = S3C24XX_SERIAL_MINOR,
   
Doesn't this break existing systems and configurations that are
expecting 204:64 as the location of this serial port?
   
   
   I tested this on Exynos4210-Origen, Exynos5250-Arndale board, it works
   fine there. I haven't tested on any older boards.
  
  How did it work?  You are relying on some userspace tools to do this
  properly, right?  What about systems without those specific tools?
 
 Can this issue be resolved by using MODULE_ALIAS_CHARDEV()
 in the driver code?

How exactly would that work?
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: ohci-exynos: Change to use phy provided by the generic phy framework

[Greg KH]

On Wed, Nov 06, 2013 at 10:27:49AM +0900, Jingoo Han wrote:
 Change the phy provider used from the old usb phy specific to a new one
 using the generic phy framework.
 
 Signed-off-by: Jingoo Han jg1@samsung.com
 Cc: Kamil Debski k.deb...@samsung.com
 ---
 Exynos OHCI driver also uses Exynos USB2.0 PHY. Thus, I make this
 patch based-on Kamil Debski's patchset for adding Exynos USB 2.0 PHY
 driver.
 (http://www.spinics.net/lists/linux-samsung-soc/msg24104.html)
 
  drivers/usb/host/ohci-exynos.c |   28 
  1 file changed, 8 insertions(+), 20 deletions(-)

This has some fuzz when applied to my tree, care to redo it against my
usb-next branch of the usb.git tree on git.kernel.org?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] video phy's adaptation to *generic phy framework*

[Greg KH]

On Wed, Oct 16, 2013 at 09:58:09PM +0530, Kishon Vijay Abraham I wrote:
 Hi Greg,
 
 This series includes video PHY adaptation to Generic PHY Framework.
 With the adaptation they were able to get rid of plat data callbacks.
 
 Since you've taken the Generic PHY Framework, I think this series should
 also go into your tree.

All now applied, thanks.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 2/8] usb: phy: omap-usb2: use the new generic PHY framework

[Greg KH]

On Wed, Aug 21, 2013 at 11:16:07AM +0530, Kishon Vijay Abraham I wrote:
 Used the generic PHY framework API to create the PHY. Now the power off and
 power on are done in omap_usb_power_off and omap_usb_power_on respectively.
 The omap-usb2 driver is also moved to driver/phy.
 
 However using the old USB PHY library cannot be completely removed
 because OTG is intertwined with PHY and moving to the new framework
 will break OTG. Once we have a separate OTG state machine, we
 can get rid of the USB PHY library.
 
 Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
 Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
 Acked-by: Felipe Balbi ba...@ti.com
 ---
  drivers/phy/Kconfig   |   12 +
  drivers/phy/Makefile  |1 +
  drivers/{usb = }/phy/phy-omap-usb2.c |   45 
 ++---
  drivers/usb/phy/Kconfig   |   10 
  drivers/usb/phy/Makefile  |1 -
  5 files changed, 54 insertions(+), 15 deletions(-)
  rename drivers/{usb = }/phy/phy-omap-usb2.c (88%)

I tried to apply this to my USB branch, but it fails.

Kishon, you were going to refresh this patch series, right?  Please do,
because as-is, I can't take it.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 0/8] PHY framework

[Greg KH]

On Fri, Sep 20, 2013 at 11:04:40AM +0530, Kishon Vijay Abraham I wrote:
 Hi Greg,
 
 On Tuesday 17 September 2013 09:11 PM, Felipe Balbi wrote:
  On Wed, Sep 04, 2013 at 02:27:06PM +0530, Kishon Vijay Abraham I wrote:
  On Tuesday 03 September 2013 09:20 PM, Greg KH wrote:
  On Tue, Sep 03, 2013 at 08:55:23PM +0530, Kishon Vijay Abraham I wrote:
  Hi Greg,
 
  On Wednesday 28 August 2013 12:50 AM, Felipe Balbi wrote:
  Hi,
 
  On Mon, Aug 26, 2013 at 01:44:49PM +0530, Kishon Vijay Abraham I wrote:
  On Wednesday 21 August 2013 11:16 AM, Kishon Vijay Abraham I wrote:
  Added a generic PHY framework that provides a set of APIs for the PHY 
  drivers
  to create/destroy a PHY and APIs for the PHY users to obtain a 
  reference to
  the PHY with or without using phandle.
 
  This framework will be of use only to devices that uses external PHY 
  (PHY
  functionality is not embedded within the controller).
 
  The intention of creating this framework is to bring the phy drivers 
  spread
  all over the Linux kernel to drivers/phy to increase code re-use and 
  to
  increase code maintainability.
 
  Comments to make PHY as bus wasn't done because PHY devices can be 
  part of
  other bus and making a same device attached to multiple bus leads to 
  bad
  design.
 
  If the PHY driver has to send notification on connect/disconnect, the 
  PHY
  driver should make use of the extcon framework. Using this susbsystem
  to use extcon framwork will have to be analysed.
 
  You can find this patch series @
  git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git 
  testing
 
  Looks like there are not further comments on this series. Can you take 
  this in
  your misc tree?
 
  Do you want me to queue these for you ? There are quite a few users for
  this framework already and I know of at least 2 others which will show
  up for v3.13.
 
  Can you queue this patch series? There are quite a few users already for 
  this
  framework.
 
  It will have to wait for 3.13 as the merge window for new features has
  been closed for a week or so.  Sorry, I'll queue this up after 3.12-rc1
  is out.
 
  Alright, thanks.
  
  Just a gentle ping on this one...
 
 Let me know if you want me to rebase this patch series on the latest mainline 
 HEAD.

Yes please.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 0/8] PHY framework

[Greg KH]

On Tue, Sep 03, 2013 at 08:55:23PM +0530, Kishon Vijay Abraham I wrote:
 Hi Greg,
 
 On Wednesday 28 August 2013 12:50 AM, Felipe Balbi wrote:
  Hi,
  
  On Mon, Aug 26, 2013 at 01:44:49PM +0530, Kishon Vijay Abraham I wrote:
  On Wednesday 21 August 2013 11:16 AM, Kishon Vijay Abraham I wrote:
  Added a generic PHY framework that provides a set of APIs for the PHY 
  drivers
  to create/destroy a PHY and APIs for the PHY users to obtain a reference 
  to
  the PHY with or without using phandle.
 
  This framework will be of use only to devices that uses external PHY (PHY
  functionality is not embedded within the controller).
 
  The intention of creating this framework is to bring the phy drivers 
  spread
  all over the Linux kernel to drivers/phy to increase code re-use and to
  increase code maintainability.
 
  Comments to make PHY as bus wasn't done because PHY devices can be part of
  other bus and making a same device attached to multiple bus leads to bad
  design.
 
  If the PHY driver has to send notification on connect/disconnect, the PHY
  driver should make use of the extcon framework. Using this susbsystem
  to use extcon framwork will have to be analysed.
 
  You can find this patch series @
  git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git testing
 
  Looks like there are not further comments on this series. Can you take 
  this in
  your misc tree?
  
  Do you want me to queue these for you ? There are quite a few users for
  this framework already and I know of at least 2 others which will show
  up for v3.13.
 
 Can you queue this patch series? There are quite a few users already for this
 framework.

It will have to wait for 3.13 as the merge window for new features has
been closed for a week or so.  Sorry, I'll queue this up after 3.12-rc1
is out.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework

[Greg KH]

On Fri, Jul 26, 2013 at 06:19:16PM +0530, Kishon Vijay Abraham I wrote:
 +static int phy_get_id(void)
 +{
 + int ret;
 + int id;
 +
 + ret = ida_pre_get(phy_ida, GFP_KERNEL);
 + if (!ret)
 + return -ENOMEM;
 +
 + ret = ida_get_new(phy_ida, id);
 + if (ret  0)
 + return ret;
 +
 + return id;
 +}

ida_simple_get() instead?  And if you do that, you can get rid of this
function entirely.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 6/8] usb: host: ohci-s3c2410 Use clk_prepare_enable/clk_disable_unprepare

[Greg KH]

On Wed, Jul 31, 2013 at 04:44:43PM -0400, Alan Stern wrote:
 On Wed, 31 Jul 2013, Tomasz Figa wrote:
 
  Alan, Greg,
  
  On Tuesday 23 of July 2013 01:49:23 Tomasz Figa wrote:
   This patch modifies the ohci-s3c2410 driver to prepare and unprepare
   clocks in addition to enabling and disabling, since it is required
   by common clock framework.
   
   Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
   ---
drivers/usb/host/ohci-s3c2410.c | 8 
1 file changed, 4 insertions(+), 4 deletions(-)
   
   diff --git a/drivers/usb/host/ohci-s3c2410.c
   b/drivers/usb/host/ohci-s3c2410.c index e125770..db096bf 100644
   --- a/drivers/usb/host/ohci-s3c2410.c
   +++ b/drivers/usb/host/ohci-s3c2410.c
   @@ -47,10 +47,10 @@ static void s3c2410_start_hc(struct platform_device
   *dev, struct usb_hcd *hcd)
   
 dev_dbg(dev-dev, s3c2410_start_hc:\n);
   
   - clk_enable(usb_clk);
   + clk_prepare_enable(usb_clk);
 mdelay(2);  /* let the bus clock stabilise */
   
   - clk_enable(clk);
   + clk_prepare_enable(clk);
   
 if (info != NULL) {
 info-hcd   = hcd;
   @@ -75,8 +75,8 @@ static void s3c2410_stop_hc(struct platform_device
   *dev) (info-enable_oc)(info, 0);
 }
   
   - clk_disable(clk);
   - clk_disable(usb_clk);
   + clk_disable_unprepare(clk);
   + clk_disable_unprepare(usb_clk);
}
   
/* ohci_s3c2410_hub_status_data
  
  Any chance to get your ack on this?
 
 Sorry, this must have slipped past.  It's fine with me.

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
  On Tue, 23 Jul 2013, Tomasz Figa wrote:
  
  On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
  Hi Alan,
  
  Thanks for helping to clarify the issues here.
  
  Okay.  Are PHYs _always_ platform devices?
 
  They can be i2c, spi or any other device types as well.
  
  In those other cases, presumably there is no platform data associated
  with the PHY since it isn't a platform device.  Then how does the
  kernel know which controller is attached to the PHY?  Is this spelled
  out in platform data associated with the PHY's i2c/spi/whatever parent?
 
 Yes. I think we could use i2c_board_info for passing platform data.
  
 PHY.  Currently this information is represented by name or 
  ID
 strings embedded in platform data.
 
  right. It's embedded in the platform data of the controller.
 
  It must also be embedded in the PHY's platform data somehow.
  Otherwise, how would the kernel know which PHY to use?
 
  By using a PHY lookup as Stephen and I suggested in our previous
  replies. Without any extra data in platform data. (I have even posted a
  code example.)
  
  I don't understand, because I don't know what a PHY lookup does.
 
 It is how the PHY framework finds a PHY, when the controller (say USB)requests
 a PHY from the PHY framework.
  
  In this case, it doesn't matter where the platform_device structures
  are created or where the driver source code is.  Let's take a simple
  example.  Suppose the system design includes a PHY named foo.  Then
  the board file could contain:
 
  struct phy_info { ... } phy_foo;
  EXPORT_SYMBOL_GPL(phy_foo);
 
  and a header file would contain:
 
  extern struct phy_info phy_foo;
 
  The PHY supplier could then call phy_create(phy_foo), and the PHY
  client could call phy_find(phy_foo).  Or something like that; make up
  your own structure tags and function names.
 
  It's still possible to have conflicts, but now two PHYs with the same
  name (or a misspelled name somewhere) will cause an error at link
  time.
 
  This is incorrect, sorry. First of all it's a layering violation - you
  export random driver-specific symbols from one driver to another. Then
  
  No, that's not what I said.  Neither the PHY driver nor the controller
  driver exports anything to the other.  Instead, both drivers use data
  exported by the board file.
 
 I think instead we can use the same data while creating the platform data of
 the controller and the PHY.
 The PHY driver while creating the PHY (using PHY framework) will also pass the
 *data* it actually got from the platform data to the framework.
 The PHY user driver (USB), while requesting for the PHY (from the PHY
 framework) will pass the *data* it got from its platform data.
 The PHY framework can do a comparison of the *data* pointers it has and return
 the appropriate PHY to the controller.
  
  imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
  there are two types of consumer drivers (e.g. USB host controllers). Now
  consider following mapping:
 
  SoC   PHY consumer
  A PHY1HOST1
  B PHY1HOST2
  C PHY2HOST1
  D PHY2HOST2
 
  So we have to be able to use any of the PHYs with any of the host
  drivers. This means you would have to export symbol with the same name
  from both PHY drivers, which obviously would not work in this case,
  because having both drivers enabled (in a multiplatform aware
  configuration) would lead to linking conflict.
  
  You're right; the scheme was too simple.  Instead, the board file must
  export two types of data structures, one for PHYs and one for
  controllers.  Like this:
  
  struct phy_info {
  /* Info for the controller attached to this PHY */
  struct controller_info  *hinfo;
  };
  
  struct controller_info {
  /* Info for the PHY which this controller is attached to */
  struct phy_info *pinfo;
  };
  
  The board file for SoC A would contain:
  
  struct phy_info phy1 = {host1);
  EXPORT_SYMBOL(phy1);
  struct controller_info host1 = {phy1};
  EXPORT_SYMBOL(host1);
  
  The board file for SoC B would contain:
  
  struct phy_info phy1 = {host2);
  EXPORT_SYMBOL(phy1);
  struct controller_info host2 = {phy1};
  EXPORT_SYMBOL(host2);
 
 I meant something like this
 struct phy_info {
   const char *name;
 };
 
 struct phy_platform_data {
   .
   .
   struct phy_info *info;
 };
 
 struct usb_controller_platform_data {
   .
   .
   struct phy_info *info;
 };
 
 struct phy_info phy_info;
 
 While creating the phy device
   struct phy_platform_data phy_data;
   phy_data.info = info;
   platform_device_add_data(pdev, phy_data, sizeof(*phy_data))
   platform_device_add();
 
 While creating the controller device
   struct usb_controller_platform_data controller_data;
   controller_data.info = info;
   

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 09:58:34PM +0530, Kishon Vijay Abraham I wrote:
 Hi Greg,
 
 On Tuesday 23 July 2013 09:48 PM, Greg KH wrote:
  On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
  Hi,
 
  On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
  On Tue, 23 Jul 2013, Tomasz Figa wrote:
 
  On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
  Hi Alan,
 
  Thanks for helping to clarify the issues here.
 
  Okay.  Are PHYs _always_ platform devices?
 
  They can be i2c, spi or any other device types as well.
 
  In those other cases, presumably there is no platform data associated
  with the PHY since it isn't a platform device.  Then how does the
  kernel know which controller is attached to the PHY?  Is this spelled
  out in platform data associated with the PHY's i2c/spi/whatever parent?
 .
 .
 snip
 .
 .
 
 static struct phy *phy_lookup(void *priv) {
 .
 .
 if (phy-priv==priv) //instead of string comparison, we'll use 
  pointer
 return phy;
 }
 
  PHY driver should be like
 phy_create((dev, ops, pdata-info);
 
  The controller driver would do
 phy_get(dev, NULL, pdata-info);
 
  Now the PHY framework will check for a match of *priv* pointer and return 
  the PHY.
 
  I think this should be possible?
  
  Ick, no.  Why can't you just pass the pointer to the phy itself?  If you
  had a priv pointer to search from, then you could have just passed the
  original phy pointer in the first place, right?
  
  The issue is that a string name is not going to scale at all, as it
  requires hard-coded information that will change over time (as the
  existing clock interface is already showing.)
  
  Please just pass the real phy pointer around, that's what it is there
  for.  Your board binding logic/code should be able to handle this, as
  it somehow was going to do the same thing with a name.
 
 The problem is the board file won't have the *phy* pointer. *phy* pointer is
 created at a much later time when the phy driver is probed.

Ok, then save it then, as no one could have used it before then, right?

All I don't want to see is any get by name/void * functions in the
api, as that way is fragile and will break, as people have already
shown.

Just pass the real pointer around.  If that is somehow a problem, then
something larger is a problem with how board devices are tied together :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
  Ick, no.  Why can't you just pass the pointer to the phy itself?  If you
  had a priv pointer to search from, then you could have just passed the
  original phy pointer in the first place, right?
 
 IMHO it would be better if you provided some code example, but let's try to 
 check if I understood you correctly.

It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)

 8
 
 [Board file]
 
 static struct phy my_phy;
 
 static struct platform_device phy_pdev = {
   /* ... */
   .platform_data = my_phy;
   /* ... */
 };
 
 static struct platform_device phy_pdev = {
   /* ... */
   .platform_data = my_phy;
   /* ... */
 };
 
 [Provider driver]
 
 struct phy *phy = pdev-dev.platform_data;
 
 ret = phy_create(phy);
 
 [Consumer driver]
 
 struct phy *phy = pdev-dev.platform_data;
 
 ret = phy_get(pdev-dev, phy);
 
 8
 
 Is this what you mean?

No.  Well, kind of.  What's wrong with using the platform data structure
unique to the board to have the pointer?

For example (just randomly picking one), the ata-pxa driver would change
include/linux/platform_data/ata-pxa.h to have a phy pointer in it:

struct phy;

struct  pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy  *phy;
};

Then, when you create the platform, set the phy* pointer with a call to
phy_create().  Then you can use that pointer wherever that plaform data
is available (i.e. whereever platform_data is at).

  The issue is that a string name is not going to scale at all, as it
  requires hard-coded information that will change over time (as the
  existing clock interface is already showing.)
 
 I fully agree that a simple, single string will not scale even in some, not 
 so uncommon cases, but there is already a lot of existing lookup solutions 
 over the kernel and so there is no point in introducing another one.

I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should.  I'll go tackle those other ones after this one
is taken care of, to show how the others should be handled as well.

  Please just pass the real phy pointer around, that's what it is there
  for.  Your board binding logic/code should be able to handle this, as
  it somehow was going to do the same thing with a name.
 
 It's technically correct, but quality of this solution isn't really nice, 
 because it's a layering violation (at least if I understood what you mean). 
 This is because you need to have full definition of struct phy in board file 
 and a structure that is used as private data in PHY core comes from 
 platform code.

No, just a pointer, you don't need the full structure until you get to
some .c code that actually manipulates the phy itself, for all other
places, you are just dealing with a pointer and a structure you never
reference.

Does that make more sense?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 06:44:56PM +0100, Mark Brown wrote:
 On Tue, Jul 23, 2013 at 10:37:11AM -0700, Greg KH wrote:
  On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
 
   I fully agree that a simple, single string will not scale even in some, 
   not 
   so uncommon cases, but there is already a lot of existing lookup 
   solutions 
   over the kernel and so there is no point in introducing another one.
 
  I'm trying to get _rid_ of lookup solutions and just use a real
  pointer, as you should.  I'll go tackle those other ones after this one
  is taken care of, to show how the others should be handled as well.
 
 What are the problems you are seeing with doing things with lookups?

You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back earlier in
this thread for details about that.)

 Having to write platform data for everything gets old fast and the code
 duplication is pretty tedious...

Adding a single pointer is tedious?  Where is the name that you are
going to lookup going to come from?  That code doesn't write itself...

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 07:48:11PM +0200, Tomasz Figa wrote:
 On Tuesday 23 of July 2013 10:37:11 Greg KH wrote:
  On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no.  Why can't you just pass the pointer to the phy itself?  If
you
had a priv pointer to search from, then you could have just passed
the
original phy pointer in the first place, right?
   
   IMHO it would be better if you provided some code example, but let's
   try to check if I understood you correctly.
  
  It's not my code that I want to have added, so I don't have to write
  examples, I just get to complain about the existing stuff :)
 
 Still, I think that some small code snippets illustrating the idea are 
 really helpful.
 
   8
   
   
   [Board file]
   
   static struct phy my_phy;
   
   static struct platform_device phy_pdev = {
   
 /* ... */
 .platform_data = my_phy;
 /* ... */
   
   };
   
   static struct platform_device phy_pdev = {
   
 /* ... */
 .platform_data = my_phy;
 /* ... */
   
   };
   
   [Provider driver]
   
   struct phy *phy = pdev-dev.platform_data;
   
   ret = phy_create(phy);
   
   [Consumer driver]
   
   struct phy *phy = pdev-dev.platform_data;
   
   ret = phy_get(pdev-dev, phy);
   
   ---
   -8
   
   Is this what you mean?
  
  No.  Well, kind of.  What's wrong with using the platform data structure
  unique to the board to have the pointer?
  
  For example (just randomly picking one), the ata-pxa driver would change
  include/linux/platform_data/ata-pxa.h to have a phy pointer in it:
  
  struct phy;
  
  struct  pata_pxa_pdata {
  /* PXA DMA DREQ0:2 pin */
  uint32_tdma_dreq;
  /* Register shift */
  uint32_treg_shift;
  /* IRQ flags */
  uint32_tirq_flags;
  /* PHY */
  struct phy  *phy;
  };
  
  Then, when you create the platform, set the phy* pointer with a call to
  phy_create().  Then you can use that pointer wherever that plaform data
  is available (i.e. whereever platform_data is at).
 
 Hmm? So, do you suggest to call phy_create() from board file? What phy_ops 
 struct and other hardware parameters would it take?
 
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
   
   I fully agree that a simple, single string will not scale even in some,
   not so uncommon cases, but there is already a lot of existing lookup
   solutions over the kernel and so there is no point in introducing
   another one.
  I'm trying to get _rid_ of lookup solutions and just use a real
  pointer, as you should.  I'll go tackle those other ones after this one
  is taken care of, to show how the others should be handled as well.
 
 There was a reason for introducing lookup solutions. The reason was that in 
 board file there is no way to get a pointer to something that is going to be 
 created much later in time. We don't do time travel ;-).
 
Please just pass the real phy pointer around, that's what it is
there
for.  Your board binding logic/code should be able to handle this,
as
it somehow was going to do the same thing with a name.
   
   It's technically correct, but quality of this solution isn't really
   nice, because it's a layering violation (at least if I understood what
   you mean). This is because you need to have full definition of struct
   phy in board file and a structure that is used as private data in PHY
   core comes from platform code.
  
  No, just a pointer, you don't need the full structure until you get to
  some .c code that actually manipulates the phy itself, for all other
  places, you are just dealing with a pointer and a structure you never
  reference.
  
  Does that make more sense?
 
 Well, to the point that I think I now understood your suggestion. 
 Unfortunately the suggestion alone isn't really something that can be done, 
 considering how driver core and generic frameworks work.

Ok, given that I seem to be totally confused as to exactly how the
board-specific frameworks work, I'll take your word for it.

But again, I will not accept lookup by name type solutions, when the
name is dynamic and will change.  Because you are using a name, you
can deal with a pointer, putting it _somewhere_ in your board-specific
data structures, as you are going to need to store it anyway (hint, you
had to get that name from somewhere, right?)

And maybe the way that these generic frameworks are created is wrong,
given that you don't feel that a generic pointer can be passed to the
needed devices.  That seems like a huge problem, one that has already
been pointed out is causing issues with other subsystems.

So maybe they need to be fixed?

thanks,

greg k-h

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
  You don't know the id of the device you are looking up, due to
  multiple devices being in the system (dynamic ids, look back earlier in
  this thread for details about that.)
 
 I got copied in very late so don't have most of the thread I'm afraid, 
 I did try looking at web archives but didn't see a clear problem
 statement.  In any case this is why the APIs doing lookups do the
 lookups in the context of the requesting device - devices ask for
 whatever name they use locally.

What do you mean by locally?

The problem with the api was that the phy core wanted a id and a name to
create a phy, and then later other code was doing a lookup based on
the name and id (mushed together), because it knew that this device
was the one it wanted.

Just like the clock api, which, for multiple devices, has proven to
cause problems.  I don't want to see us accept an api that we know has
issues in it now, I'd rather us fix it up properly.

Subsystems should be able to create ids how ever they want to, and not
rely on the code calling them to specify the names of the devices that
way, otherwise the api is just too fragile.

I think, that if you create a device, then just carry around the pointer
to that device (in this case a phy) and pass it to whatever other code
needs it.  No need to do lookups on known names or anything else, just
normal pointers, with no problems for multiple devices, busses, or
naming issues.

   Having to write platform data for everything gets old fast and the code
   duplication is pretty tedious...
 
  Adding a single pointer is tedious?  Where is the name that you are
  going to lookup going to come from?  That code doesn't write itself...
 
 It's adding platform data in the first place that gets tedious - and of
 course there's also DT and ACPI to worry about, it's not just a case of
 platform data and then you're done.  Pushing the lookup into library
 code means that drivers don't have to worry about any of this stuff.

I agree, so just pass around the pointer to the phy and all is good.  No
need to worry about DT or ACPI or anything else.

 For most of the APIs doing this there is a clear and unambiguous name in
 the hardware that can be used (and for hardware process reasons is
 unlikely to get changed).  The major exception to this is the clock API
 since it is relatively rare to have clear, segregated IP level
 information for IPs baked into larger chips.  The other APIs tend to be
 establishing chip to chip links.

The clock api is having problems with multiple names due to dynamic
devices from what I was told.  I want to prevent the PHY interface from
having that same issue.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 10:07:52PM +0200, Tomasz Figa wrote:
 On Tuesday 23 of July 2013 12:44:23 Greg KH wrote:
  On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back earlier
in
this thread for details about that.)
   
   I got copied in very late so don't have most of the thread I'm afraid,
   I did try looking at web archives but didn't see a clear problem
   statement.  In any case this is why the APIs doing lookups do the
   lookups in the context of the requesting device - devices ask for
   whatever name they use locally.
  
  What do you mean by locally?
  
  The problem with the api was that the phy core wanted a id and a name to
  create a phy, and then later other code was doing a lookup based on
  the name and id (mushed together), because it knew that this device
  was the one it wanted.
  
  Just like the clock api, which, for multiple devices, has proven to
  cause problems.  I don't want to see us accept an api that we know has
  issues in it now, I'd rather us fix it up properly.
  
  Subsystems should be able to create ids how ever they want to, and not
  rely on the code calling them to specify the names of the devices that
  way, otherwise the api is just too fragile.
  
  I think, that if you create a device, then just carry around the pointer
  to that device (in this case a phy) and pass it to whatever other code
  needs it.  No need to do lookups on known names or anything else,
  just normal pointers, with no problems for multiple devices, busses, or
  naming issues.
 
 PHY object is not a device, it is something that a device driver creates 
 (one or more instances of) when it is being probed.

But you created a 'struct device' for it, so I think of it as a device
be it virtual or real :)

 You don't have a clean way to export this PHY object to other driver,
 other than keeping this PHY on a list inside PHY core with some
 well-known ID (e.g. device name + consumer port name/index, like in
 regulator core) and then to use this well-known ID inside consumer
 driver as a lookup key passed to phy_get();
 
 Actually I think for PHY case, exactly the same way as used for
 regulators might be completely fine:
 
 1. Each PHY would have some kind of platform, non-unique name, that is 
 just used to print some messages (like the platform/board name of a 
 regulator).
 2. Each PHY would have an array of consumers. Consumer specifier would 
 consist of consumer device name and consumer port name - just like in 
 regulator subsystem.
 3. PHY driver receives an array of, let's say, phy_init_data inside its 
 platform data that it would use to register its PHYs.
 4. Consumer drivers would have constant consumer port names and wouldn't 
 receive any information about PHYs from platform code.
 
 Code example:
 
 [Board file]
 
 static const struct phy_consumer_data usb_20_phy0_consumers[] = {
   {
   .devname = foo-ehci,
   .port = usbphy,
   },
 };
 
 static const struct phy_consumer_data usb_20_phy1_consumers[] = {
   {
   .devname = foo-otg,
   .port = otgphy,
   },
 };
 
 static const struct phy_init_data my_phys[] = {
   {
   .name = USB 2.0 PHY 0,
   .consumers = usb_20_phy0_consumers,
   .num_consumers = ARRAY_SIZE(usb_20_phy0_consumers),
   },
   {
   .name = USB 2.0 PHY 1,
   .consumers = usb_20_phy1_consumers,
   .num_consumers = ARRAY_SIZE(usb_20_phy1_consumers),
   },
   { }
 };
 
 static const struct platform_device usb_phy_pdev = {
   .name = foo-usbphy,
   .id = -1,
   .dev = {
   .platform_data = my_phys,
   },
 };
 
 [PHY driver]
 
 static int foo_usbphy_probe(pdev)
 {
   struct foo_usbphy *foo;
   struct phy_init_data *init_data = pdev-dev.platform_data;
   /* ... */
   // for each PHY in init_data {
   phy_register(foo-phy[i], init_data[i]);
   // }
   /* ... */
 }
 
 [EHCI driver]
 
 static int foo_ehci_probe(pdev)
 {
   struct phy *phy;
   /* ... */
   phy = phy_get(pdev-dev, usbphy);
   /* ... */
 }
 
 [OTG driver]
 
 static int foo_otg_probe(pdev)
 {
   struct phy *phy;
   /* ... */
   phy = phy_get(pdev-dev, otgphy);
   /* ... */
 }

That's not so bad, as long as you let the phy core use whatever name it
wants for the device when it registers it with sysfs.  Use the name you
are requesting as a tag or some such hint as to what the phy can be
looked up by.

Good luck handling duplicate tags :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 11:05:48PM +0200, Tomasz Figa wrote:
  That's not so bad, as long as you let the phy core use whatever name it
  wants for the device when it registers it with sysfs.
 
 Yes, in regulator core consumer names are completely separated from this. 
 Regulator core simply assigns a sequential integer ID to each regulator 
 and registers /sys/class/regulator/regulator.ID for each regulator.

Yes, that's fine.

  Use the name you
  are requesting as a tag or some such hint as to what the phy can be
  looked up by.
  
  Good luck handling duplicate tags :)
 
 The tag alone is not a key. Lookup key consists of two components, 
 consumer device name and consumer tag. What kind of duplicate tags can be 
 a problem here?

Ok, I didn't realize it looked at both parts, that makes sense, thanks.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Mon, Jul 22, 2013 at 12:55:18PM +0530, Kishon Vijay Abraham I wrote:
  The issue (or one of the issues) in this discussion is that 
  Greg does not like the idea of using names or IDs to associate
  PHYs with controllers, because they are too prone to
  duplications or other errors.  Pointers are more reliable.
  
  But pointers to what?  Since the only data known to be 
  available to both the PHY driver and controller driver is the
  platform data, the obvious answer is a pointer to platform data
  (either for the PHY or for the controller, or maybe both).
 
 hmm.. it's not going to be simple though as the platform device for the PHY 
 and
 controller can be created in entirely different places. e.g., in some cases 
 the
 PHY device is a child of some mfd core device (the device will be created in
 drivers/mfd) and the controller driver (usually) is created in board file. I
 guess then we have to come up with something to share a pointer in two
 different files.

What's wrong with using the platform_data structure that is unique to
all boards (see include/platform_data/ for examples)?  Isn't that what
this structure is there for?

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Sun, Jul 21, 2013 at 01:12:07PM +0200, Tomasz Figa wrote:
 On Sunday 21 of July 2013 16:37:33 Kishon Vijay Abraham I wrote:
  Hi,
  
  On Sunday 21 July 2013 04:01 PM, Tomasz Figa wrote:
   Hi,
   
   On Saturday 20 of July 2013 19:59:10 Greg KH wrote:
   On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
   On Sat, 20 Jul 2013, Greg KH wrote:
   That should be passed using platform data.
   
   Ick, don't pass strings around, pass pointers.  If you have
   platform
   data you can get to, then put the pointer there, don't use a
   name.
   
   I don't think I understood you here :-s We wont have phy pointer
   when we create the device for the controller no?(it'll be done in
   board file). Probably I'm missing something.
   
   Why will you not have that pointer?  You can't rely on the name
   as
   the device id will not match up, so you should be able to rely on
   the pointer being in the structure that the board sets up, right?
   
   Don't use names, especially as ids can, and will, change, that is
   going
   to cause big problems.  Use pointers, this is C, we are supposed to
   be
   doing that :)
   
   Kishon, I think what Greg means is this:  The name you are using
   must
   be stored somewhere in a data structure constructed by the board
   file,
   right?  Or at least, associated with some data structure somehow.
   Otherwise the platform code wouldn't know which PHY hardware
   corresponded to a particular name.
   
   Greg's suggestion is that you store the address of that data
   structure
   in the platform data instead of storing the name string.  Have the
   consumer pass the data structure's address when it calls phy_create,
   instead of passing the name.  Then you don't have to worry about two
   PHYs accidentally ending up with the same name or any other similar
   problems.
   
   Close, but the issue is that whatever returns from phy_create()
   should
   then be used, no need to call any find functions, as you can just
   use
   the pointer that phy_create() returns.  Much like all other class api
   functions in the kernel work.
   
   I think there is a confusion here about who registers the PHYs.
   
   All platform code does is registering a platform/i2c/whatever device,
   which causes a driver (located in drivers/phy/) to be instantiated.
   Such drivers call phy_create(), usually in their probe() callbacks,
   so platform_code has no way (and should have no way, for the sake of
   layering) to get what phy_create() returns.

Why not put pointers in the platform data structure that can hold these
pointers?  I thought that is why we created those structures in the
first place.  If not, what are they there for?

   IMHO we need a lookup method for PHYs, just like for clocks,
   regulators, PWMs or even i2c busses because there are complex cases
   when passing just a name using platform data will not work. I would
   second what Stephen said [1] and define a structure doing things in a
   DT-like way.
   
   Example;
   
   [platform code]
   
   static const struct phy_lookup my_phy_lookup[] = {
   
 PHY_LOOKUP(s3c-hsotg.0, otg, samsung-usbphy.1, phy.2),
  
  The only problem here is that if *PLATFORM_DEVID_AUTO* is used while
  creating the device, the ids in the device name would change and
  PHY_LOOKUP wont be useful.
 
 I don't think this is a problem. All the existing lookup methods already 
 use ID to identify devices (see regulators, clkdev, PWMs, i2c, ...). You 
 can simply add a requirement that the ID must be assigned manually, 
 without using PLATFORM_DEVID_AUTO to use PHY lookup.

And I'm saying that this idea, of using a specific name and id, is
frought with fragility and will break in the future in various ways when
devices get added to systems, making these strings constantly have to be
kept up to date with different board configurations.

People, NEVER, hardcode something like an id.  The fact that this
happens today with the clock code, doesn't make it right, it makes the
clock code wrong.  Others have already said that this is wrong there as
well, as systems change and dynamic ids get used more and more.

Let's not repeat the same mistakes of the past just because we refuse to
learn from them...

So again, the find a phy by a string functions should be removed, the
device id should be automatically created by the phy core just to make
things unique in sysfs, and no driver code should _ever_ be reliant on
the number that is being created, and the pointer to the phy structure
should be used everywhere instead.

With those types of changes, I will consider merging this subsystem, but
without them, sorry, I will not.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Sun, Jul 21, 2013 at 12:22:48PM +0200, Sascha Hauer wrote:
 On Sat, Jul 20, 2013 at 07:59:10PM -0700, Greg KH wrote:
  On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
   On Sat, 20 Jul 2013, Greg KH wrote:
   
 That should be passed using platform data.
 
 Ick, don't pass strings around, pass pointers.  If you have platform
 data you can get to, then put the pointer there, don't use a name.
 
 I don't think I understood you here :-s We wont have phy pointer
 when we create the device for the controller no?(it'll be done in
 board file). Probably I'm missing something.

Why will you not have that pointer?  You can't rely on the name as the
device id will not match up, so you should be able to rely on the
pointer being in the structure that the board sets up, right?

Don't use names, especially as ids can, and will, change, that is going
to cause big problems.  Use pointers, this is C, we are supposed to be
doing that :)
   
   Kishon, I think what Greg means is this:  The name you are using must
   be stored somewhere in a data structure constructed by the board file,
   right?  Or at least, associated with some data structure somehow.  
   Otherwise the platform code wouldn't know which PHY hardware
   corresponded to a particular name.
   
   Greg's suggestion is that you store the address of that data structure 
   in the platform data instead of storing the name string.  Have the 
   consumer pass the data structure's address when it calls phy_create, 
   instead of passing the name.  Then you don't have to worry about two 
   PHYs accidentally ending up with the same name or any other similar 
   problems.
  
  Close, but the issue is that whatever returns from phy_create() should
  then be used, no need to call any find functions, as you can just use
  the pointer that phy_create() returns.  Much like all other class api
  functions in the kernel work.
 
 I think the problem here is to connect two from the bus structure
 completely independent devices. Several frameworks (ASoC, soc-camera)
 had this problem and this wasn't solved until the advent of devicetrees
 and their phandles.
 phy_create might be called from the probe function of some i2c device
 (the phy device) and the resulting pointer is then needed in some other
 platform devices (the user of the phy) probe function.
 The best solution we have right now is implemented in the clk framework
 which uses a string matching of the device names in clk_get() (at least
 in the non-dt case).

I would argue that clocks are wrong here as well, as others have already
pointed out.

What's wrong with the platform_data structure, why can't that be used
for this?

Or, if not, we can always add pointers to the platform device structure,
or even the main 'struct device' as well, that's what it is there for.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Sat, Jul 20, 2013 at 08:49:32AM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Saturday 20 July 2013 05:20 AM, Greg KH wrote:
 On Fri, Jul 19, 2013 at 12:06:01PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Friday 19 July 2013 11:59 AM, Greg KH wrote:
 On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Friday 19 July 2013 11:13 AM, Greg KH wrote:
 On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote:
 +   ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
 
 Your naming is odd, no phy anywhere in it?  You rely on the sender 
 to
 never send a duplicate name.id pair?  Why not create your own ids 
 based
 on the number of phys in the system, like almost all other classes 
 and
 subsystems do?
 
 hmm.. some PHY drivers use the id they provide to perform some of 
 their
 internal operation as in [1] (This is used only if a single PHY 
 provider
 implements multiple PHYS). Probably I'll add an option like 
 PLATFORM_DEVID_AUTO
 to give the PHY drivers an option to use auto id.
 
 [1] -
 http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
 
 No, who cares about the id?  No one outside of the phy core ever 
 should,
 because you pass back the only pointer that they really do care about,
 if they need to do anything with the device.  Use that, and then you 
 can
 
 hmm.. ok.
 
 rip out all of the search for a phy by a string logic, as that's not
 
 Actually this is needed for non-dt boot case. In the case of dt boot, 
 we use a
 phandle by which the controller can get a reference to the phy. But in 
 the case
 of non-dt boot, the controller can get a reference to the phy only by 
 label.
 
 I don't understand.  They registered the phy, and got back a pointer to
 it.  Why can't they save it in their local structure to use it again
 later if needed?  They should never have to ask for the device, as the
 
 One is a *PHY provider* driver which is a driver for some PHY device. 
 This will
 use phy_create to create the phy.
 The other is a *PHY consumer* driver which might be any controller driver 
 (can
 be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference 
 to the
 phy (by *phandle* in the case of dt boot and *label* in the case of 
 non-dt boot).
 device id might be unknown if there are multiple devices in the system.
 
 I agree with you on the device id part. That need not be known to the PHY 
 driver.
 
 How does a consumer know which label to use in a non-dt system if
 there are multiple PHYs in the system?
 
 That should be passed using platform data.
 
 Ick, don't pass strings around, pass pointers.  If you have platform
 data you can get to, then put the pointer there, don't use a name.
 
 I don't think I understood you here :-s We wont have phy pointer
 when we create the device for the controller no?(it'll be done in
 board file). Probably I'm missing something.

Why will you not have that pointer?  You can't rely on the name as the
device id will not match up, so you should be able to rely on the
pointer being in the structure that the board sets up, right?

Don't use names, especially as ids can, and will, change, that is going
to cause big problems.  Use pointers, this is C, we are supposed to be
doing that :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
 On Sat, 20 Jul 2013, Greg KH wrote:
 
   That should be passed using platform data.
   
   Ick, don't pass strings around, pass pointers.  If you have platform
   data you can get to, then put the pointer there, don't use a name.
   
   I don't think I understood you here :-s We wont have phy pointer
   when we create the device for the controller no?(it'll be done in
   board file). Probably I'm missing something.
  
  Why will you not have that pointer?  You can't rely on the name as the
  device id will not match up, so you should be able to rely on the
  pointer being in the structure that the board sets up, right?
  
  Don't use names, especially as ids can, and will, change, that is going
  to cause big problems.  Use pointers, this is C, we are supposed to be
  doing that :)
 
 Kishon, I think what Greg means is this:  The name you are using must
 be stored somewhere in a data structure constructed by the board file,
 right?  Or at least, associated with some data structure somehow.  
 Otherwise the platform code wouldn't know which PHY hardware
 corresponded to a particular name.
 
 Greg's suggestion is that you store the address of that data structure 
 in the platform data instead of storing the name string.  Have the 
 consumer pass the data structure's address when it calls phy_create, 
 instead of passing the name.  Then you don't have to worry about two 
 PHYs accidentally ending up with the same name or any other similar 
 problems.

Close, but the issue is that whatever returns from phy_create() should
then be used, no need to call any find functions, as you can just use
the pointer that phy_create() returns.  Much like all other class api
functions in the kernel work.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Friday 19 July 2013 11:13 AM, Greg KH wrote:
  On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote:
  +  ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
 
  Your naming is odd, no phy anywhere in it?  You rely on the sender to
  never send a duplicate name.id pair?  Why not create your own ids based
  on the number of phys in the system, like almost all other classes and
  subsystems do?
 
  hmm.. some PHY drivers use the id they provide to perform some of their
  internal operation as in [1] (This is used only if a single PHY provider
  implements multiple PHYS). Probably I'll add an option like 
  PLATFORM_DEVID_AUTO
  to give the PHY drivers an option to use auto id.
 
  [1] -
  http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
 
  No, who cares about the id?  No one outside of the phy core ever should,
  because you pass back the only pointer that they really do care about,
  if they need to do anything with the device.  Use that, and then you can
 
  hmm.. ok.
 
  rip out all of the search for a phy by a string logic, as that's not
 
  Actually this is needed for non-dt boot case. In the case of dt boot, we 
  use a
  phandle by which the controller can get a reference to the phy. But in the 
  case
  of non-dt boot, the controller can get a reference to the phy only by 
  label.
  
  I don't understand.  They registered the phy, and got back a pointer to
  it.  Why can't they save it in their local structure to use it again
  later if needed?  They should never have to ask for the device, as the
 
 One is a *PHY provider* driver which is a driver for some PHY device. This 
 will
 use phy_create to create the phy.
 The other is a *PHY consumer* driver which might be any controller driver (can
 be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to the
 phy (by *phandle* in the case of dt boot and *label* in the case of non-dt 
 boot).
  device id might be unknown if there are multiple devices in the system.
 
 I agree with you on the device id part. That need not be known to the PHY 
 driver.

How does a consumer know which label to use in a non-dt system if
there are multiple PHYs in the system?

Do you have any drivers that are non-dt using this yet?

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Fri, Jul 19, 2013 at 12:06:01PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Friday 19 July 2013 11:59 AM, Greg KH wrote:
  On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote:
  Hi,
 
  On Friday 19 July 2013 11:13 AM, Greg KH wrote:
  On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote:
  +ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
 
  Your naming is odd, no phy anywhere in it?  You rely on the sender 
  to
  never send a duplicate name.id pair?  Why not create your own ids 
  based
  on the number of phys in the system, like almost all other classes and
  subsystems do?
 
  hmm.. some PHY drivers use the id they provide to perform some of their
  internal operation as in [1] (This is used only if a single PHY 
  provider
  implements multiple PHYS). Probably I'll add an option like 
  PLATFORM_DEVID_AUTO
  to give the PHY drivers an option to use auto id.
 
  [1] -
  http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
 
  No, who cares about the id?  No one outside of the phy core ever should,
  because you pass back the only pointer that they really do care about,
  if they need to do anything with the device.  Use that, and then you can
 
  hmm.. ok.
 
  rip out all of the search for a phy by a string logic, as that's not
 
  Actually this is needed for non-dt boot case. In the case of dt boot, we 
  use a
  phandle by which the controller can get a reference to the phy. But in 
  the case
  of non-dt boot, the controller can get a reference to the phy only by 
  label.
 
  I don't understand.  They registered the phy, and got back a pointer to
  it.  Why can't they save it in their local structure to use it again
  later if needed?  They should never have to ask for the device, as the
 
  One is a *PHY provider* driver which is a driver for some PHY device. This 
  will
  use phy_create to create the phy.
  The other is a *PHY consumer* driver which might be any controller driver 
  (can
  be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to 
  the
  phy (by *phandle* in the case of dt boot and *label* in the case of non-dt 
  boot).
  device id might be unknown if there are multiple devices in the system.
 
  I agree with you on the device id part. That need not be known to the PHY 
  driver.
  
  How does a consumer know which label to use in a non-dt system if
  there are multiple PHYs in the system?
 
 That should be passed using platform data.

Ick, don't pass strings around, pass pointers.  If you have platform
data you can get to, then put the pointer there, don't use a name.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
 +struct phy_provider *__of_phy_provider_register(struct device *dev,
 + struct module *owner, struct phy * (*of_xlate)(struct device *dev,
 + struct of_phandle_args *args));
 +struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
 + struct module *owner, struct phy * (*of_xlate)(struct device *dev,
 + struct of_phandle_args *args))
 +
 +__of_phy_provider_register and __devm_of_phy_provider_register can be used to
 +register the phy_provider and it takes device, owner and of_xlate as
 +arguments. For the dt boot case, all PHY providers should use one of the 
 above
 +2 APIs to register the PHY provider.

Why do you have __ for the prefix of a public function?  Is that really
the way that OF handles this type of thing?

 --- /dev/null
 +++ b/drivers/phy/Kconfig
 @@ -0,0 +1,13 @@
 +#
 +# PHY
 +#
 +
 +menuconfig GENERIC_PHY
 + tristate PHY Subsystem
 + help
 +   Generic PHY support.
 +
 +   This framework is designed to provide a generic interface for PHY
 +   devices present in the kernel. This layer will have the generic
 +   API by which phy drivers can create PHY using the phy framework and
 +   phy users can obtain reference to the PHY.

Again, please reverse this.  The drivers that use it should select it,
not depend on it, which will then enable this option.  I will never know
if I need to enable it, and based on your follow-on patches, if I don't,
drivers that were working just fine, now disappeared from my build,
which isn't nice, and a pain to notice and fix up.

 +/**
 + * phy_create() - create a new phy
 + * @dev: device that is creating the new phy
 + * @id: id of the phy
 + * @ops: function pointers for performing phy operations
 + * @label: label given to the phy
 + *
 + * Called to create a phy using phy framework.
 + */
 +struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
 + const char *label)
 +{
 + int ret;
 + struct phy *phy;
 +
 + if (!dev) {
 + dev_WARN(dev, no device provided for PHY\n);
 + ret = -EINVAL;
 + goto err0;
 + }
 +
 + phy = kzalloc(sizeof(*phy), GFP_KERNEL);
 + if (!phy) {
 + ret = -ENOMEM;
 + goto err0;
 + }
 +
 + device_initialize(phy-dev);
 + mutex_init(phy-mutex);
 +
 + phy-dev.class = phy_class;
 + phy-dev.parent = dev;
 + phy-dev.of_node = dev-of_node;
 + phy-id = id;
 + phy-ops = ops;
 + phy-label = kstrdup(label, GFP_KERNEL);
 +
 + ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);

Your naming is odd, no phy anywhere in it?  You rely on the sender to
never send a duplicate name.id pair?  Why not create your own ids based
on the number of phys in the system, like almost all other classes and
subsystems do?

 +static inline int phy_pm_runtime_get(struct phy *phy)
 +{
 + if (WARN(IS_ERR(phy), Invalid PHY reference\n))
 + return -EINVAL;

Why would phy ever not be valid and a error pointer?  And why dump the
stack if that happens, that seems really extreme.

 +
 + if (!pm_runtime_enabled(phy-dev))
 + return -ENOTSUPP;
 +
 + return pm_runtime_get(phy-dev);
 +}

This, and the other inline functions in this .h file seem huge, why are
they inline and not in the .c file?  There's no speed issues, and it
should save space overall in the .c file.  Please move them.


 +static inline int phy_init(struct phy *phy)
 +{
 + int ret;
 +
 + ret = phy_pm_runtime_get_sync(phy);
 + if (ret  0  ret != -ENOTSUPP)
 + return ret;
 +
 + mutex_lock(phy-mutex);
 + if (phy-init_count++ == 0  phy-ops-init) {
 + ret = phy-ops-init(phy);
 + if (ret  0) {
 + dev_err(phy-dev, phy init failed -- %d\n, ret);
 + goto out;
 + }
 + }
 +
 +out:
 + mutex_unlock(phy-mutex);
 + phy_pm_runtime_put(phy);
 + return ret;
 +}
 +
 +static inline int phy_exit(struct phy *phy)
 +{
 + int ret;
 +
 + ret = phy_pm_runtime_get_sync(phy);
 + if (ret  0  ret != -ENOTSUPP)
 + return ret;
 +
 + mutex_lock(phy-mutex);
 + if (--phy-init_count == 0  phy-ops-exit) {
 + ret = phy-ops-exit(phy);
 + if (ret  0) {
 + dev_err(phy-dev, phy exit failed -- %d\n, ret);
 + goto out;
 + }
 + }
 +
 +out:
 + mutex_unlock(phy-mutex);
 + phy_pm_runtime_put(phy);
 + return ret;
 +}
 +
 +static inline int phy_power_on(struct phy *phy)
 +{
 + int ret = -ENOTSUPP;
 +
 + ret = phy_pm_runtime_get_sync(phy);
 + if (ret  0  ret != -ENOTSUPP)
 + return ret;
 +
 + mutex_lock(phy-mutex);
 + if (phy-power_count++ == 0  phy-ops-power_on) {
 + ret = phy-ops-power_on(phy);
 + if (ret  0) {
 + dev_err(phy-dev, phy 

Re: [PATCH 02/15] usb: phy: omap-usb2: use the new generic PHY framework

[Greg KH]

On Thu, Jul 18, 2013 at 12:16:11PM +0530, Kishon Vijay Abraham I wrote:
 Used the generic PHY framework API to create the PHY. Now the power off and
 power on are done in omap_usb_power_off and omap_usb_power_on respectively.
 
 However using the old USB PHY library cannot be completely removed
 because OTG is intertwined with PHY and moving to the new framework
 will break OTG. Once we have a separate OTG state machine, we
 can get rid of the USB PHY library.
 
 Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
 Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
 Acked-by: Felipe Balbi ba...@ti.com
 ---
  drivers/usb/phy/Kconfig |1 +
  drivers/usb/phy/phy-omap-usb2.c |   45 
 +++
  2 files changed, 42 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
 index 3622fff..cc55993 100644
 --- a/drivers/usb/phy/Kconfig
 +++ b/drivers/usb/phy/Kconfig
 @@ -75,6 +75,7 @@ config OMAP_CONTROL_USB
  config OMAP_USB2
   tristate OMAP USB2 PHY Driver
   depends on ARCH_OMAP2PLUS
 + depends on GENERIC_PHY
   select OMAP_CONTROL_USB
   help
 Enable this to support the transceiver that is part of SOC. This
 diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
 index 844ab68..751b30f 100644
 --- a/drivers/usb/phy/phy-omap-usb2.c
 +++ b/drivers/usb/phy/phy-omap-usb2.c
 @@ -28,6 +28,7 @@
  #include linux/pm_runtime.h
  #include linux/delay.h
  #include linux/usb/omap_control_usb.h
 +#include linux/phy/phy.h
  
  /**
   * omap_usb2_set_comparator - links the comparator present in the sytem with
 @@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int 
 suspend)
   return 0;
  }
  
 +static int omap_usb_power_off(struct phy *x)
 +{
 + struct omap_usb *phy = phy_get_drvdata(x);
 +
 + omap_control_usb_phy_power(phy-control_dev, 0);
 +
 + return 0;
 +}
 +
 +static int omap_usb_power_on(struct phy *x)
 +{
 + struct omap_usb *phy = phy_get_drvdata(x);
 +
 + omap_control_usb_phy_power(phy-control_dev, 1);
 +
 + return 0;
 +}
 +
 +static struct phy_ops ops = {
 + .power_on   = omap_usb_power_on,
 + .power_off  = omap_usb_power_off,
 + .owner  = THIS_MODULE,
 +};
 +
  static int omap_usb2_probe(struct platform_device *pdev)
  {
   struct omap_usb *phy;
 + struct phy  *generic_phy;
   struct usb_otg  *otg;
 + struct phy_provider *phy_provider;
  
   phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
   if (!phy) {
 @@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
   phy-phy.otg= otg;
   phy-phy.type   = USB_PHY_TYPE_USB2;
  
 + phy_provider = devm_of_phy_provider_register(phy-dev,
 + of_phy_simple_xlate);
 + if (IS_ERR(phy_provider))
 + return PTR_ERR(phy_provider);
 +
   phy-control_dev = omap_get_control_dev();
   if (IS_ERR(phy-control_dev)) {
   dev_dbg(pdev-dev, Failed to get control device\n);
 @@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
   otg-start_srp  = omap_usb_start_srp;
   otg-phy= phy-phy;
  
 + platform_set_drvdata(pdev, phy);
 + pm_runtime_enable(phy-dev);
 +
 + generic_phy = devm_phy_create(phy-dev, 0, ops, omap-usb2);
 + if (IS_ERR(generic_phy))
 + return PTR_ERR(generic_phy);

So, if I have two of these controllers in my system, I can't create the
second phy because the name for it will be identical to the first?
That's why the phy core should handle the id, and not rely on the
drivers to set it, as they have no idea how many they have in the
system.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote:
  +ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
 
  Your naming is odd, no phy anywhere in it?  You rely on the sender to
  never send a duplicate name.id pair?  Why not create your own ids based
  on the number of phys in the system, like almost all other classes and
  subsystems do?
 
  hmm.. some PHY drivers use the id they provide to perform some of their
  internal operation as in [1] (This is used only if a single PHY provider
  implements multiple PHYS). Probably I'll add an option like 
  PLATFORM_DEVID_AUTO
  to give the PHY drivers an option to use auto id.
 
  [1] -
  http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
  
  No, who cares about the id?  No one outside of the phy core ever should,
  because you pass back the only pointer that they really do care about,
  if they need to do anything with the device.  Use that, and then you can
 
 hmm.. ok.
 
  rip out all of the search for a phy by a string logic, as that's not
 
 Actually this is needed for non-dt boot case. In the case of dt boot, we use a
 phandle by which the controller can get a reference to the phy. But in the 
 case
 of non-dt boot, the controller can get a reference to the phy only by label.

I don't understand.  They registered the phy, and got back a pointer to
it.  Why can't they save it in their local structure to use it again
later if needed?  They should never have to ask for the device, as the
device id might be unknown if there are multiple devices in the system.

Or am I missing something?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] usb: dwc3-exynos: Fix compatible strings for the device

[Greg KH]

On Thu, Jan 24, 2013 at 07:15:30PM +0530, Vivek Gautam wrote:
 Using specific chip in compatible strings. Newer SOCs can claim
 device by using older string in the compatible list.
 
 Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
 Acked-by: Grant Likely grant.lik...@secretlab.ca
 Reviewed-by: Doug Anderson diand...@chromium.org
 ---
  drivers/usb/dwc3/dwc3-exynos.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

Felipe, want me to take this one, or will you?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] serial: samsung: Remove NULL checking for baud clock

[Greg KH]

On Wed, Jun 20, 2012 at 01:28:36PM +0900, Kukjin Kim wrote:
 Kukjin Kim wrote:
  
  Kyoungil Kim wrote:
  
   Kyoungil Kim wrote:
  
   I missed following.
  
   Russell King suggested:
  
   As I said, drivers have no business interpreting anything but
 IS_ERR(clk)
   as being an error.  They should not make any other
   assumptions.
  
   Suggested-by: Russell King rmk+ker...@arm.linux.org.uk
  
Signed-off-by: Kyoungil Kim ki0351@samsung.com
---
 drivers/tty/serial/samsung.c |   21 -
 1 files changed, 12 insertions(+), 9 deletions(-)
   
diff --git a/drivers/tty/serial/samsung.c
  b/drivers/tty/serial/samsung.c
index d8b0aee..5668538 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -529,7 +529,7 @@ static void s3c24xx_serial_pm(struct uart_port
  *port,
   unsigned int level,
   
switch (level) {
case 3:
-   if (!IS_ERR(ourport-baudclk)  ourport-baudclk != 
NULL)
+   if (!IS_ERR(ourport-baudclk))
clk_disable(ourport-baudclk);
   
clk_disable(ourport-clk);
@@ -538,7 +538,7 @@ static void s3c24xx_serial_pm(struct uart_port
  *port,
   unsigned int level,
case 0:
clk_enable(ourport-clk);
   
-   if (!IS_ERR(ourport-baudclk)  ourport-baudclk != 
NULL)
+   if (!IS_ERR(ourport-baudclk))
clk_enable(ourport-baudclk);
   
break;
@@ -604,7 +604,6 @@ static unsigned int s3c24xx_serial_getclk(struct
   s3c24xx_uart_port *ourport,
char clkname[MAX_CLK_NAME_LENGTH];
int calc_deviation, deviation = (1  30) - 1;
   
-   *best_clk = NULL;
clk_sel = (ourport-cfg-clk_sel) ? ourport-cfg-clk_sel :
ourport-info-def_clk_sel;
for (cnt = 0; cnt  info-num_clks; cnt++) {
@@ -613,7 +612,7 @@ static unsigned int s3c24xx_serial_getclk(struct
   s3c24xx_uart_port *ourport,
   
sprintf(clkname, clk_uart_baud%d, cnt);
clk = clk_get(ourport-port.dev, clkname);
-   if (IS_ERR_OR_NULL(clk))
+   if (IS_ERR(clk))
continue;
   
rate = clk_get_rate(clk);
@@ -684,7 +683,7 @@ static void s3c24xx_serial_set_termios(struct
   uart_port *port,
 {
struct s3c2410_uartcfg *cfg = s3c24xx_port_to_cfg(port);
struct s3c24xx_uart_port *ourport = to_ourport(port);
-   struct clk *clk = NULL;
+   struct clk *clk = ERR_PTR(-EINVAL);
unsigned long flags;
unsigned int baud, quot, clk_sel = 0;
unsigned int ulcon;
@@ -705,7 +704,7 @@ static void s3c24xx_serial_set_termios(struct
   uart_port *port,
quot = s3c24xx_serial_getclk(ourport, baud, clk, clk_sel);
if (baud == 38400  (port-flags  UPF_SPD_MASK) == 
UPF_SPD_CUST)
quot = port-custom_divisor;
-   if (!clk)
+   if (IS_ERR(clk))
return;
   
/* check to see if we need  to change clock source */
@@ -713,9 +712,9 @@ static void s3c24xx_serial_set_termios(struct
   uart_port *port,
if (ourport-baudclk != clk) {
s3c24xx_serial_setsource(port, clk_sel);
   
-   if (ourport-baudclk != NULL  
!IS_ERR(ourport-baudclk)) {
+   if (!IS_ERR(ourport-baudclk)) {
clk_disable(ourport-baudclk);
-   ourport-baudclk  = NULL;
+   ourport-baudclk = ERR_PTR(-EINVAL);
}
   
clk_enable(clk);
@@ -1160,6 +1159,9 @@ static ssize_t s3c24xx_serial_show_clksrc(struct
   device *dev,
struct uart_port *port = s3c24xx_dev_to_port(dev);
struct s3c24xx_uart_port *ourport = to_ourport(port);
   
+   if (IS_ERR(ourport-baudclk))
+   return -EINVAL;
+
return snprintf(buf, PAGE_SIZE, * %s\n, 
ourport-baudclk-name);
 }
   
@@ -1200,6 +1202,7 @@ static int s3c24xx_serial_probe(struct
   platform_device *pdev)
return -ENODEV;
}
   
+   ourport-baudclk = ERR_PTR(-EINVAL);
ourport-info = ourport-drv_data-info;
ourport-cfg = (pdev-dev.platform_data) ?
(struct s3c2410_uartcfg 
*)pdev-dev.platform_data :
@@ -1387,7 +1390,7 @@ s3c24xx_serial_get_options(struct uart_port
  *port,
   int *baud,
sprintf(clk_name, clk_uart_baud%d, clk_sel);
   
clk = clk_get(port-dev, clk_name);
-   if (!IS_ERR(clk)  clk != NULL)
+   if (!IS_ERR(clk))
rate = clk_get_rate(clk);
else
  

Re: [PATCH 2/2] serial: samsung: Fixed wrong comparison for baudclk_rate

[Greg KH]

On Wed, Jun 20, 2012 at 01:28:40PM +0900, Kukjin Kim wrote:
 Kyoungil Kim wrote:
  
  port-baudclk_rate should be compared to the rate of port-baudclk,
  because port-baudclk_rate was assigned as the rate of port-baudclk
  previously.
  So to check that the current baudclk rate is same as previous rate,
  the target of comparison sholud be the rate of port-baudclk.
  
  Signed-off-by: Jun-Ho, Yoon junho78.y...@samsung.com
  Signed-off-by: Kyoungil Kim ki0351@samsung.com
 
 (Cc'ed Greg)
 
 Acked-by: Kukjin Kim kgene@samsung.com
 
 Greg, this is needed to fix the wrong comparison for baudclk_rate for
 Samsung Serial.
 Could you please pick this up in your fix tree?

Already there, thanks.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] serial: samsung: protect NULL dereference of clock name

[Greg KH]

On Wed, Jun 20, 2012 at 01:28:47PM +0900, Kukjin Kim wrote:
 Kyoungil Kim wrote:
  
  From: KeyYoung Park keyyoung.p...@samsung.com
  
  When priting the serial clock source, if clock source name is null,
  kernel reference NULL point.
  
  Signed-off-by: KeyYoung Park keyyoung.p...@samsung.com
  Signed-off-by: Huisung Kang hs1218.k...@samsung.com
  Signed-off-by: Kyoungil Kim ki0351@samsung.com
 
 (Cc'ed Greg)
 
 Acked-by: Kukjin Kim kgene@samsung.com
 
 Greg, could you please pick this up?

Already have, thanks.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] Samsung usb stuff for v3.5

[Greg KH]

On Thu, May 17, 2012 at 03:45:55PM +0900, Kukjin Kim wrote:
 Hi Greg and Felipe,
 
 Please pull Samsung 's3c-hsotg' UDC support for EXYNOS4210 and S5PV210 from:
   git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
 v3.5-for-usb
 
 Since it has a dependency on 'usb: hsotg: samsung ...' patches which have
 been already in usb tree now, so would be better if you could pull this
 series in your tree. Or this can be sent to upstream in the end of upcoming
 merge window after pulling usb tree. But I think usb tree is better in this
 case.
 
 If any problems, please kindly let me know.

All pulled into my usb-next branch now, thanks.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] USB: SET SEL request definition

[Greg KH]

On Mon, Feb 06, 2012 at 05:12:33PM +0900, Anton Tikhomirov wrote:
 Cc: Kukjin Kim kgene.kim at samsung.com
 Cc: Greg Kroah-Hartman gregkh at suse.de
 Cc: Felipe Balbi balbi at ti.com

What is that mess?  It belongs, with real email addresses, below your
signed-off-by line, if at all.

 Adds SET SEL standard request definition as defined by ch9
 of the USB3.0 specification.
 
 Signed-off-by: Anton Tikhomirov av.tikhomi...@samsung.com
 ---
  include/linux/usb/ch9.h |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
 index 61b2905..76ff771 100644
 --- a/include/linux/usb/ch9.h
 +++ b/include/linux/usb/ch9.h
 @@ -88,6 +88,7 @@
  #define USB_REQ_GET_INTERFACE0x0A
  #define USB_REQ_SET_INTERFACE0x0B
  #define USB_REQ_SYNCH_FRAME  0x0C
 +#define USB_REQ_SET_SEL  0x30
  
  #define USB_REQ_SET_ENCRYPTION   0x0D/* Wireless USB */
  #define USB_REQ_GET_ENCRYPTION   0x0E

Why did you insert this out of order?

greg please note my email address change as well k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re: [PATCH 0/3] Support Samsung S5P OHCI device and driver

[Greg KH]

On Wed, Dec 21, 2011 at 01:38:24PM +0900, Kukjin Kim wrote:
 
 
  -Original Message-
  From: Greg KH [mailto:g...@kroah.com]
  Sent: Saturday, December 10, 2011 8:58 AM
  To: Kukjin Kim
  Cc: 'Jingoo Han'; linux-arm-ker...@lists.infradead.org; linux-
  u...@vger.kernel.org; linux-samsung-soc@vger.kernel.org; gre...@suse.de;
  st...@rowland.harvard.edu; 'Yulgon Kim'; 'Shim Joonyoung'
  Subject: Re: Re: [PATCH 0/3] Support Samsung S5P OHCI device and driver
  
  On Thu, Dec 01, 2011 at 07:10:17PM +0900, Kukjin Kim wrote:
   Greg KH wrote:
   
On Mon, Nov 21, 2011 at 11:57:16AM +, Jingoo Han wrote:
  -Original Message-
  From: Greg KH [mailto:g...@kroah.com]
  Sent: Friday, November 18, 2011 4:12 AM
  To: Jingoo Han
  Cc: linux-arm-ker...@lists.infradead.org; linux-
  u...@vger.kernel.org;
  linux-samsung-soc@vger.kernel.org; gre...@suse.de;
  st...@rowland.harvard.edu; kgene@samsung.com;
yulgon@samsung.com;
  jy0922.s...@samsung.com
  Subject: Re: [PATCH 0/3] Support Samsung S5P OHCI device and
  driver
 
  On Tue, Nov 15, 2011 at 06:49:58AM +, Jingoo Han wrote:
   Hello.
  
   This patch series adds USB OHCI device and initial driver for
Samsung
   S5P SoCs and is based from Linux 3.2-rc1. I have tested on
  SMDKV310
  board
   using EXYNOS4.
 
  This is to be sent through some arm tree, not the usb tree, right?
 Right, 1st and 2nd patch are sent throught arm tree.
 However, 3rd patch is sent through the usb tree.
 Thanks.
 [PATCH 1/3] ARM: SAMSUNG: Add USB OHCI device
 [PATCH 2/3]  ARM: EXYNOS: Add USB OHCI support to SMDKV310 board
 [PATCH 3/3] USB: Add S5P OHCI diver
   
   Hi Greg,
  
   Sorry for late response. I came back today from family vacation...
  
   Anyway,
  
So the third patch does not depend on the first two?  If it does, that
means I will get build errors in the USB tree when I apply that patch,
right?
   
If so, then all three should go through a single tree.
   
   Yes, right. This series would be handled in same tree so that we can
  avoid
   useless conflicts.
  
   I commented small thing on 3rd patch. So I think, if you're ok on his
   updated patch, it can be sent to upstream via Samsung tree. Or your tree
   with my ack?
  
  The samsung tree is fine to take these, thanks.
  
 Hi Greg,
 
 OK, let me pick this up in my tree...so, when I apply, may I can have a ack
 from you on 3rd patch?

Yes, feel free to add:
Acked-by: Greg Kroah-Hartman gre...@suse.de

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re: [PATCH 0/3] Support Samsung S5P OHCI device and driver

[Greg KH]

On Thu, Dec 01, 2011 at 07:10:17PM +0900, Kukjin Kim wrote:
 Greg KH wrote:
  
  On Mon, Nov 21, 2011 at 11:57:16AM +, Jingoo Han wrote:
-Original Message-
From: Greg KH [mailto:g...@kroah.com]
Sent: Friday, November 18, 2011 4:12 AM
To: Jingoo Han
Cc: linux-arm-ker...@lists.infradead.org; linux-...@vger.kernel.org;
linux-samsung-soc@vger.kernel.org; gre...@suse.de;
st...@rowland.harvard.edu; kgene@samsung.com;
  yulgon@samsung.com;
jy0922.s...@samsung.com
Subject: Re: [PATCH 0/3] Support Samsung S5P OHCI device and driver
   
On Tue, Nov 15, 2011 at 06:49:58AM +, Jingoo Han wrote:
 Hello.

 This patch series adds USB OHCI device and initial driver for
  Samsung
 S5P SoCs and is based from Linux 3.2-rc1. I have tested on SMDKV310
board
 using EXYNOS4.
   
This is to be sent through some arm tree, not the usb tree, right?
   Right, 1st and 2nd patch are sent throught arm tree.
   However, 3rd patch is sent through the usb tree.
   Thanks.
   [PATCH 1/3] ARM: SAMSUNG: Add USB OHCI device
   [PATCH 2/3]  ARM: EXYNOS: Add USB OHCI support to SMDKV310 board
   [PATCH 3/3] USB: Add S5P OHCI diver
  
 Hi Greg,
 
 Sorry for late response. I came back today from family vacation...
 
 Anyway,
 
  So the third patch does not depend on the first two?  If it does, that
  means I will get build errors in the USB tree when I apply that patch,
  right?
  
  If so, then all three should go through a single tree.
  
 Yes, right. This series would be handled in same tree so that we can avoid
 useless conflicts.
 
 I commented small thing on 3rd patch. So I think, if you're ok on his
 updated patch, it can be sent to upstream via Samsung tree. Or your tree
 with my ack?

The samsung tree is fine to take these, thanks.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re: [PATCH 0/3] Support Samsung S5P OHCI device and driver

[Greg KH]

On Mon, Nov 21, 2011 at 11:57:16AM +, Jingoo Han wrote:
  -Original Message-
  From: Greg KH [mailto:g...@kroah.com]
  Sent: Friday, November 18, 2011 4:12 AM
  To: Jingoo Han
  Cc: linux-arm-ker...@lists.infradead.org; linux-...@vger.kernel.org;
  linux-samsung-soc@vger.kernel.org; gre...@suse.de;
  st...@rowland.harvard.edu; kgene@samsung.com; yulgon@samsung.com;
  jy0922.s...@samsung.com
  Subject: Re: [PATCH 0/3] Support Samsung S5P OHCI device and driver
  
  On Tue, Nov 15, 2011 at 06:49:58AM +, Jingoo Han wrote:
   Hello.
  
   This patch series adds USB OHCI device and initial driver for Samsung
   S5P SoCs and is based from Linux 3.2-rc1. I have tested on SMDKV310
  board
   using EXYNOS4.
  
  This is to be sent through some arm tree, not the usb tree, right?
 Right, 1st and 2nd patch are sent throught arm tree.
 However, 3rd patch is sent through the usb tree.
 Thanks.
 [PATCH 1/3] ARM: SAMSUNG: Add USB OHCI device
 [PATCH 2/3]  ARM: EXYNOS: Add USB OHCI support to SMDKV310 board
 [PATCH 3/3] USB: Add S5P OHCI diver

So the third patch does not depend on the first two?  If it does, that
means I will get build errors in the USB tree when I apply that patch,
right?

If so, then all three should go through a single tree.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] Support Samsung S5P OHCI device and driver

[Greg KH]

On Tue, Nov 15, 2011 at 06:49:58AM +, Jingoo Han wrote:
 Hello.
 
 This patch series adds USB OHCI device and initial driver for Samsung
 S5P SoCs and is based from Linux 3.2-rc1. I have tested on SMDKV310 board
 using EXYNOS4.

This is to be sent through some arm tree, not the usb tree, right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/9] serial: samsung: rework clock lookup and add device tree support

[Greg KH]

 
 
 Thomas Abraham (9):
serial: samsung: Keep a copy of the location of platform data in driver's 
  private data
serial: samsung: move handling of fclk/n clock to platform code
serial: samsung: switch to clkdev based clock lookup
serial: samsung: remove struct 's3c24xx_uart_clksrc' and all uses of it
serial: samsung: remove all uses of get_clksrc and set_clksrc
arm: samsung: register uart clocks to clock lookup list
serial: samsung: merge all SoC specific port reset functions
serial: samsung: merge probe() function from all SoC specific extensions
serial: samsung: add device tree support
 
   .../devicetree/bindings/serial/samsung_uart.txt|   14 +
   arch/arm/mach-exynos4/clock.c  |  106 ++--
   arch/arm/mach-exynos4/init.c   |   21 +-
   arch/arm/mach-s3c2410/mach-bast.c  |   22 -
   arch/arm/mach-s3c2410/mach-vr1000.c|   24 -
   arch/arm/mach-s3c2410/s3c2410.c|6 +
   arch/arm/mach-s3c2412/clock.c  |7 +
   arch/arm/mach-s3c2440/clock.c  |   44 ++
   arch/arm/mach-s3c2440/mach-anubis.c|   22 +-
   arch/arm/mach-s3c2440/mach-at2440evb.c |   22 +-
   arch/arm/mach-s3c2440/mach-osiris.c|   24 +-
   arch/arm/mach-s3c2440/mach-rx1950.c|   18 +-
   arch/arm/mach-s3c2440/mach-rx3715.c|   19 +-
   arch/arm/mach-s3c64xx/clock.c  |   37 +-
   arch/arm/mach-s5p64x0/clock-s5p6440.c  |   32 +-
   arch/arm/mach-s5p64x0/clock-s5p6450.c  |   32 +-
   arch/arm/mach-s5p64x0/init.c   |   31 -
   arch/arm/mach-s5pc100/clock.c  |   33 +-
   arch/arm/mach-s5pv210/clock.c  |  107 ++--
   arch/arm/mach-s5pv210/init.c   |   19 -
   arch/arm/plat-s3c24xx/s3c2443-clock.c  |   23 +-
   arch/arm/plat-samsung/include/plat/regs-serial.h   |   45 +-
   drivers/tty/serial/Kconfig |   45 +--
   drivers/tty/serial/Makefile|5 -
   drivers/tty/serial/s3c2410.c   |  115 
   drivers/tty/serial/s3c2412.c   |  149 -
   drivers/tty/serial/s3c2440.c   |  178 --
   drivers/tty/serial/s3c6400.c   |  149 -
   drivers/tty/serial/s5pv210.c   |  158 -
   drivers/tty/serial/samsung.c   |  639 
  
   drivers/tty/serial/samsung.h   |   32 +-
   31 files changed, 752 insertions(+), 1426 deletions(-)
   create mode 100644 
  Documentation/devicetree/bindings/serial/samsung_uart.txt
   delete mode 100644 drivers/tty/serial/s3c2410.c
   delete mode 100644 drivers/tty/serial/s3c2412.c
   delete mode 100644 drivers/tty/serial/s3c2440.c
   delete mode 100644 drivers/tty/serial/s3c6400.c
   delete mode 100644 drivers/tty/serial/s5pv210.c
 
 (Cc'ed Greg KH)
 
 Looks good for me, and I need to get the ack from Greg before applying.
 
 Greg, if you're ok, I'd like to send this series to upstream via
 Samsung tree for supporting device tree because this touches a lot
 of arch/arm/ Samsung stuff. If any problems, please let me know.

No objection from me, feel free to take this through your tree.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] Samsung Fixes for v3.1-rc7

[Greg KH]

On Fri, Sep 16, 2011 at 08:50:51PM +0900, Kukjin Kim wrote:
 Arnd Bergmann wrote:
  
  On Thursday 15 September 2011, Kukjin Kim wrote:
   This is Samsung fixes for v3.1
  
   Please pull from:
 git://github.com/kgene/linux-samsung.git samsung-fixes-2
   As you know, git.kernel.org/master.kernel.org has been down so I use
   temporary git repo. at github now.
  
   These things are needed for v3.1 and if any problems, please let me
 know.
  
   As a note, others for v3.2 will be sent in the next week...
  
  Thanks, pulled.
  
  Is it correct that you want none of these patches to be backported
  into the stable or longterm releases? Some of these look like they
  should be marked 'Cc: sta...@kernel.org'.
  
 (Cc'ed Greg K-H)
 
 Yes, you're right. Some patches are needed to sent to sta...@kernel.org.
 But unfortunately, when they have been submitted, there was no 'Cc:
 sta...@kernel.org'...

What do you mean?  Do you mean you just forgot to add them, or you
created them so long ago before there was a sta...@kernel.org?

 In this case, I'm not sure which following method is proper...
 - to send 'pull request' to Greg / sta...@kernel.org like bug fix during -rc
 - to submit each patches with adding 'Cc: sta...@kernel.org' again
 - or ?

Are these in Linus's tree already?  If so, send me the git commit ids
and I will add them to the stable kernels.

If not, wait until they are, and then send me the git commit ids, and I
will then add them.

Before they get to Linus, there's nothing I can do with them, and I
don't accept pull requests as that makes no sense when it comes to the
stable kernel patch flow.

Does this help?

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: SAMSUNG: serial: Fix on handling of one clock source for UART

[Greg KH]

On Fri, May 27, 2011 at 07:04:03PM -0700, Kukjin Kim wrote:
 From: Boojin Kim boojin@samsung.com
 
 This patch fixes the way of comparison for handling of two or more
 clock sources for UART.
 
 For example, if just only one clock source is defined even though
 there are two clock sources for UART, the serial driver does not
 set proper clock up. Of course, it is problem.
 
 So this patch changes the condition of comparison to avoid useless
 setup clock and adds a flag 'NO_NEED_CHECK_CLKSRC' which means
 selection of source clock is not required.
 
 In addition, since the Exynos4210 has only one clock source for UART
 this patch adds the flag into its common_init_uarts().
 
 Signed-off-by: Boojin Kim boojin@samsung.com
 Cc: Greg Kroah-Hartman g...@kroah.com
 Signed-off-by: Kukjin Kim kgene@samsung.com
 ---
  arch/arm/mach-exynos4/init.c |1 +
  arch/arm/plat-samsung/include/plat/regs-serial.h |2 ++
  drivers/tty/serial/s5pv210.c |4 ++--
  3 files changed, 5 insertions(+), 2 deletions(-)

Is this needed in older kernels as well (like .39)?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] USB: Gadget: Add Samsung S3C24XX USB High-Speed controller driver

[Greg KH]

On Sat, Apr 16, 2011 at 12:00:31PM +0200, Heiko Stübner wrote:
 Am Donnerstag 14 April 2011, 19:15:23 schrieb Alan Stern:
  On Thu, 14 Apr 2011, Greg KH wrote:
   On Thu, Apr 14, 2011 at 11:35:43AM -0400, Alan Stern wrote:
On Thu, 14 Apr 2011, Heiko [iso-8859-1] St?bner wrote:
 From: Thomas Abraham thomas...@samsung.com
 
 The Samsung's S3C2416, S3C2443 and S3C2450 includes a USB High-Speed
 device controller module. This driver enables support for USB
 high-speed gadget functionality for the Samsung S3C24xx SoC's that
 include this controller.
 
 Signed-off-by: Thomas Abraham thomas...@samsung.com
 Signed-off-by: Sangbeom Kim sbki...@samsung.com
 Signed-off-by: Kukjin Kim kgene@samsung.com
 Signed-off-by: Alexander Neumann alexan...@bumpern.de
 Signed-off-by: Heiko Stuebner he...@sntech.de

...

 +static struct usb_ep_ops s3c_hsudc_ep_ops = {
 + .enable = s3c_hsudc_ep_enable,
 + .disable = s3c_hsudc_ep_disable,
 + .alloc_request = s3c_hsudc_alloc_request,
 + .free_request = s3c_hsudc_free_request,
 + .queue = s3c_hsudc_queue,
 + .dequeue = s3c_hsudc_dequeue,
 + .set_halt = s3c_hsudc_set_halt,
 +};

There's no .set_wedge method.  Why do people always leave this out?
   
   Does the code spit out a nasty warning if this isn't set?  If not, I
   would suggest adding it so that this doesn't keep happening.
   
   Or just refuse to be able to register the structure, that would stop it
   right away :)
  
  In fact, set_wedge is optional.  But it's so easy to implement, there's
  no good reason for leaving it out.
 
 It seems Thomas [original author of the driver] will be able to implement 
 said 
 set_wedge function for it.
 As he will need a bit of time for this, two possible ways for going forward 
 come to mind:
 (1) use current driver [as set_wedge is optional] and add it later via patch
 (2) resubmit whole driver again when set_wedge is added to it
 
 Obviously I would prefer option 1 :-), but in the end it's your decision.

It shouldn't take that much time to do this, what is the delay?

I'd prefer to get the correct version implemented and would not like to
accept a patch that everyone knows is wrong.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND v3 3/4] USB: Gadget: Add Samsung S3C24XX USB High-Speed controller driver

[Greg KH]

On Wed, Mar 23, 2011 at 10:39:28PM +0100, Heiko Stübner wrote:
 From: Thomas Abraham thomas...@samsung.com
 
 The Samsung's S3C2416, S3C2443 and S3C2450 includes a USB High-Speed
 device controller module. This driver enables support for USB high-speed
 gadget functionality for the Samsung S3C24xx SoC's that include this
 controller.
 
 Signed-off-by: Thomas Abraham thomas...@samsung.com
 Signed-off-by: Sangbeom Kim sbki...@samsung.com
 Signed-off-by: Kukjin Kim kgene@samsung.com
 ---
 Changes since v1:
 - Addressed comments from Ben Dooks
 
 Changes since v2:
 - Update the driver to match the data structure changes introduced by
   commit b0fca50f5a94a268ed02cfddf8051ed9343f.
   Alexander Neumann alexan...@debian.org
 - Move s3c_hsudc to 0x29 in usb_gadget_controller_number in gadget_chips.h
   Heiko Stuebner he...@sntech.de

The Renesas driver got number 0x29, so this patch no longer applies :(

Care to respin this whole series against the next linux-next tree and
resend it to me so I can apply it?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ARM: EXYNOS4: Add usb ehci device to the NURI board

[Greg KH]

On Fri, Apr 08, 2011 at 01:22:11PM +0900, Joonyoung Shim wrote:
 This patch is to support usb ehci device to the NURI board.
 
 Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

Note, I had to apply this one by hand, I don't know what tree you made
it against.  Hopefully the merge issues will be simple to handle, as
they were pretty obvious to me.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: USB: Gadget: COMPOSITE: Debug interface number

[Greg KH]

On Fri, Feb 04, 2011 at 06:50:27PM +0900, Jassi Brar wrote:
 Hi,
 
 The commit 'avoid access beyond array max length' incorrectly
 compares w_index with MAX_CONFIG_INTERFACES. Whereas only
 lower 8bits of w_index refer to the interface number.
 So, use 'intf' rather than 'w_index' for comparison.

Why did you attach the patch in base64 mode?  Please fix your email
client to not do this and send it to me in a format that I can apply it
in.

Try using 'git send-email' instead.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: s3c-hsotg patches

[Greg KH]

On Wed, Jul 14, 2010 at 11:39:50AM +0200, Marek Szyprowski wrote:
 Hello,
 
 On Wednesday, July 07, 2010 5:13 PM Greg KH wrote:
 
  On Wed, Jul 07, 2010 at 01:02:12AM +0100, Ben Dooks wrote:
   Re-send of previous set, with corrected patches.
  
  Hm, what tree are these going through?  Do you need me to take them
  through the linux-usb tree?  Or are they going through a
  platform-specific one somewhere?
  
  confused,
 
 Looks that Ben is busy and has no time to answer...

Then we all should be too busy to apply them :)

Seriously, if a simple question like this is ignored, what happens if a
real problem is reported?

 I'm interested in merging these patches too. These patches were
 intended to be merged to 2.6.35-rc5. They are available on 
 git://git.fluff.org/bjdooks/linux for-2635-rc5/hsotg branch.
 
 IMHO these patches should go though the linux-usb tree - they
 don't touch any platform specific code, and register definitions 
 (arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h) in fact belongs
 to the driver rather than the platform. s3c-hsotg driver belongs to
 the linux-usb tree.

Ok, I can take them all, care to resend them through email?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] USB: s3c_hsotg: Add support for external USB clock

[Greg KH]

On Tue, May 25, 2010 at 05:36:48AM +0100, Ben Dooks wrote:
 From: Maurus Cuelenaere mcuelena...@gmail.com
 
 The PLL that drives the USB clock supports 3 input clocks: 12, 24 and 48Mhz.
 This patch adds support to the USB driver for setting the correct register bit
 according to the given clock.
 
 This depends on the following patch:
 [PATCH] ARM: S3C64XX: Add USB external clock definition

Care to take this through the arm tree then?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html