Re: [PATCH v7 0/6] Add ChromeOS EC CEC Support

2018-06-11 Thread Lee Jones
On Fri, 08 Jun 2018, Hans Verkuil wrote:
> On 08/06/18 10:17, Neil Armstrong wrote:
> > On 08/06/2018 09:53, Hans Verkuil wrote:
> >> On 06/01/2018 10:19 AM, Neil Armstrong wrote:
> >>> Hi All,
> >>>
> >>> The new Google "Fizz" Intel-based ChromeOS device is gaining CEC support
> >>> through it's Embedded Controller, to enable the Linux CEC Core to 
> >>> communicate
> >>> with it and get the CEC Physical Address from the correct HDMI Connector, 
> >>> the
> >>> following must be added/changed:
> >>> - Add the CEC sub-device registration in the ChromeOS EC MFD Driver
> >>> - Add the CEC related commands and events definitions into the EC MFD 
> >>> driver
> >>> - Add a way to get a CEC notifier with it's (optional) connector name
> >>> - Add the CEC notifier to the i915 HDMI driver
> >>> - Add the proper ChromeOS EC CEC Driver
> >>>
> >>> The CEC notifier with the connector name is the tricky point, since even 
> >>> on
> >>> Device-Tree platforms, there is no way to distinguish between multiple 
> >>> HDMI
> >>> connectors from the same DRM driver. The solution I implemented is pretty
> >>> simple and only adds an optional connector name to eventually distinguish
> >>> an HDMI connector notifier from another if they share the same device.
> >>
> >> This looks good to me, which brings me to the next question: how to merge
> >> this?
> >>
> >> It touches on three subsystems (media, drm, mfd), so that makes this
> >> tricky.
> >>
> >> I think there are two options: either the whole series goes through the
> >> media tree, or patches 1+2 go through drm and 3-6 through media. If there
> >> is a high chance of conflicts in the mfd code, then it is also an option to
> >> have patches 3-6 go through the mfd subsystem.
> > 
> > I think patches 3-6 should go in the mfd tree, Lee is used to handle this,
> > then I think the rest could go in the media tree.
> > 
> > Lee, do you think it would be possible to have an immutable branch with 
> > patches 3-6 ?
> > 
> > Could we have an immutable branch from media tree with patch 1 to be merged 
> > in
> > the i915 tree for patch 2 ?
> > 
> > Or patch 1+2 could me merged into the i915 tree and generate an immutable 
> > branch
> 
> I think patches 1+2 can just go to the i915 tree. The i915 driver changes 
> often,
> so going through that tree makes sense. The cec-notifier code is unlikely to 
> change,
> and I am fine with that patch going through i915.
> 
> > for media to merge the mfd branch + patch 7 ?
> 
> Patch 7? I only count 6?
> 
> If 1+2 go through drm and 3-6 go through mfd, then media isn't affected at 
> all.
> There is chance of a conflict when this is eventually pushed to mainline for
> the media Kconfig, but that's all.

What are the *build* dependencies within the set?

I'd be happy to send out a pull-request for either all of the patches,
or just the MFD changes once I've had chance to review them.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog


Re: [PATCH 1/7] i2c: i2c-gpio: move header to platform_data

2018-04-20 Thread Lee Jones
On Thu, 19 Apr 2018, Wolfram Sang wrote:

> This header only contains platform_data. Move it to the proper directory.
> 
> Signed-off-by: Wolfram Sang <w...@the-dreams.de>
> ---
>  MAINTAINERS  | 2 +-
>  arch/arm/mach-ks8695/board-acs5k.c   | 2 +-
>  arch/arm/mach-omap1/board-htcherald.c| 2 +-
>  arch/arm/mach-pxa/palmz72.c  | 2 +-
>  arch/arm/mach-pxa/viper.c| 2 +-
>  arch/arm/mach-sa1100/simpad.c| 2 +-
>  arch/mips/alchemy/board-gpr.c| 2 +-
>  drivers/i2c/busses/i2c-gpio.c| 2 +-
>  drivers/media/platform/marvell-ccic/mmp-driver.c | 2 +-
>  drivers/mfd/sm501.c  | 2 +-
>  include/linux/{ => platform_data}/i2c-gpio.h | 0
>  11 files changed, 10 insertions(+), 10 deletions(-)
>  rename include/linux/{ => platform_data}/i2c-gpio.h (100%)

Acked-by: Lee Jones <lee.jo...@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog


[PATCH 1/2] [media] rc-core: Add inlined stubs for core rc_* functions

2017-04-05 Thread Lee Jones
Currently users have to use all sorts of ugly #ifery within
their drivers in order to avoid linking issues at build time.
This patch allows users to safely call these functions when
!CONFIG_RC_CORE and make decisions based on the return value
instead.  This is a much more common and clean way of doing
things within the Linux kernel.

Signed-off-by: Lee Jones <lee.jo...@linaro.org>
---
 include/media/rc-core.h | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 73ddd721..45ba739 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -209,7 +209,14 @@ struct rc_dev {
  * @rc_driver_type: specifies the type of the RC output to be allocated
  * returns a pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 struct rc_dev *rc_allocate_device(enum rc_driver_type);
+#else
+static inline struct rc_dev *rc_allocate_device(int unused)
+{
+   return NULL;
+}
+#endif
 
 /**
  * devm_rc_allocate_device - Managed RC device allocation
@@ -218,21 +225,42 @@ struct rc_dev *rc_allocate_device(enum rc_driver_type);
  * @rc_driver_type: specifies the type of the RC output to be allocated
  * returns a pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 struct rc_dev *devm_rc_allocate_device(struct device *dev, enum 
rc_driver_type);
+#else
+static inline struct rc_dev *devm_rc_allocate_device(struct device *dev, int 
unused)
+{
+   return NULL;
+}
+#endif
 
 /**
  * rc_free_device - Frees a RC device
  *
  * @dev: pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 void rc_free_device(struct rc_dev *dev);
+#else
+static inline void rc_free_device(struct rc_dev *dev)
+{
+   return;
+}
+#endif
 
 /**
  * rc_register_device - Registers a RC device
  *
  * @dev: pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 int rc_register_device(struct rc_dev *dev);
+#else
+static inline int rc_register_device(struct rc_dev *dev)
+{
+   return -EOPNOTSUPP;
+}
+#endif
 
 /**
  * devm_rc_register_device - Manageded registering of a RC device
@@ -240,14 +268,28 @@ int rc_register_device(struct rc_dev *dev);
  * @parent: pointer to struct device.
  * @dev: pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 int devm_rc_register_device(struct device *parent, struct rc_dev *dev);
+#else
+static inline int devm_rc_register_device(struct device *parent, struct rc_dev 
*dev)
+{
+   return -EOPNOTSUPP;
+}
+#endif
 
 /**
  * rc_unregister_device - Unregisters a RC device
  *
  * @dev: pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 void rc_unregister_device(struct rc_dev *dev);
+#else
+static inline void rc_unregister_device(struct rc_dev *dev)
+{
+   return;
+}
+#endif
 
 /**
  * rc_open - Opens a RC device
-- 
2.9.3



[PATCH 2/2] [media] cec: Handle RC capability more elegantly

2017-04-05 Thread Lee Jones
If a user specifies the use of RC as a capability, they should
really be enabling RC Core code.  If they do not we WARN() them
of this and disable the capability for them.

Once we know RC Core code has not been enabled, we can update
the user's capabilities and use them as a term of reference for
other RC-only calls.  This is preferable to having ugly #ifery
scattered throughout C code.

Most of the functions are actually safe to call, since they
sensibly check for a NULL RC pointer before they attempt to
deference it.

Signed-off-by: Lee Jones <lee.jo...@linaro.org>
---
 drivers/media/cec/cec-core.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index cfe414a..c859dcf 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -208,9 +208,15 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
return ERR_PTR(-EINVAL);
if (WARN_ON(!available_las || available_las > CEC_MAX_LOG_ADDRS))
return ERR_PTR(-EINVAL);
+
+   /* If RC Core is not available, remove driver-level capability */
+   if (!IS_REACHABLE(CONFIG_RC_CORE))
+   caps &= ~CEC_CAP_RC;
+
adap = kzalloc(sizeof(*adap), GFP_KERNEL);
if (!adap)
return ERR_PTR(-ENOMEM);
+
strlcpy(adap->name, name, sizeof(adap->name));
adap->phys_addr = CEC_PHYS_ADDR_INVALID;
adap->log_addrs.cec_version = CEC_OP_CEC_VERSION_2_0;
@@ -237,7 +243,6 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
if (!(caps & CEC_CAP_RC))
return adap;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Prepare the RC input device */
adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
if (!adap->rc) {
@@ -264,9 +269,7 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
adap->rc->priv = adap;
adap->rc->map_name = RC_MAP_CEC;
adap->rc->timeout = MS_TO_NS(100);
-#else
-   adap->capabilities &= ~CEC_CAP_RC;
-#endif
+
return adap;
 }
 EXPORT_SYMBOL_GPL(cec_allocate_adapter);
@@ -285,7 +288,6 @@ int cec_register_adapter(struct cec_adapter *adap,
adap->owner = parent->driver->owner;
adap->devnode.dev.parent = parent;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
if (adap->capabilities & CEC_CAP_RC) {
adap->rc->dev.parent = parent;
res = rc_register_device(adap->rc);
@@ -298,15 +300,13 @@ int cec_register_adapter(struct cec_adapter *adap,
return res;
}
}
-#endif
 
res = cec_devnode_register(>devnode, adap->owner);
if (res) {
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Note: rc_unregister also calls rc_free */
rc_unregister_device(adap->rc);
adap->rc = NULL;
-#endif
+
return res;
}
 
@@ -337,11 +337,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
if (IS_ERR_OR_NULL(adap))
return;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Note: rc_unregister also calls rc_free */
rc_unregister_device(adap->rc);
adap->rc = NULL;
-#endif
+
debugfs_remove_recursive(adap->cec_dir);
cec_devnode_unregister(>devnode);
 }
@@ -357,9 +356,7 @@ void cec_delete_adapter(struct cec_adapter *adap)
kthread_stop(adap->kthread);
if (adap->kthread_config)
kthread_stop(adap->kthread_config);
-#if IS_REACHABLE(CONFIG_RC_CORE)
rc_free_device(adap->rc);
-#endif
kfree(adap);
 }
 EXPORT_SYMBOL_GPL(cec_delete_adapter);
-- 
2.9.3



Re: [PATCH 2/2] [media] cec: Handle RC capability more elegantly

2017-04-05 Thread Lee Jones
On Tue, 04 Apr 2017, Russell King - ARM Linux wrote:

> On Tue, Apr 04, 2017 at 05:10:05PM +0100, Lee Jones wrote:
> > @@ -237,7 +241,6 @@ struct cec_adapter *cec_allocate_adapter(const struct 
> > cec_adap_ops *ops,
> > if (!(caps & CEC_CAP_RC))
> > return adap;
> >  
> > -#if IS_REACHABLE(CONFIG_RC_CORE)
> > /* Prepare the RC input device */
> > adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> > if (!adap->rc) {
> 
> The above, coupled with patch 1:
> 
> +#ifdef CONFIG_RC_CORE
>  struct rc_dev *rc_allocate_device(enum rc_driver_type);
> +#else
> +static inline struct rc_dev *rc_allocate_device(int unused)
> +{
> +   return ERR_PTR(-EOPNOTSUPP);
> +}
> +#endif
> 
> is really not nice.  You claim that this is how stuff is done elsewhere
> in the kernel, but no, it isn't.  Look at debugfs.

I'm afraid you have entered half way through a conversation, which as
caused a misunderstanding.  Apologies for not being clear in my commit
log.

When I say "this is how it's done else where", that is in reference to
offering stubs which can be tucked away in a header file, rather than
being forced to #if out any functionality which is not available.

> You're right that debugfs returns an error pointer when it's not
> configured.  However, the debugfs dentry is only ever passed back into
> the debugfs APIs, it is never dereferenced by the caller.

Continued on from my last point:

What I do not mean is that this solution is perfect and does not
require a review.  You are completely correct in what you say, the
return values I have used are not suitable.  I failed to see how
callers were treating the return value.  I will carry out due
diligence on that point and re-submit as per your request.

> That is not the case here.  The effect if your change is that the
> following dereferences will oops the kernel.  This is unacceptable for
> a feature that is deconfigured.

Fair point Russell.  Thanks, will fix.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog


Re: [PATCH] [media] cec: Handle RC capability more elegantly

2017-04-05 Thread Lee Jones
On Tue, 04 Apr 2017, Russell King - ARM Linux wrote:

> On Tue, Apr 04, 2017 at 04:19:39PM +0100, Lee Jones wrote:
> > On Tue, 04 Apr 2017, Hans Verkuil wrote:
> > 
> > > On 04/04/2017 04:43 PM, Lee Jones wrote:
> > > > If a user specifies the use of RC as a capability, they should
> > > > really be enabling RC Core code.  If they do not we WARN() them
> > > > of this and disable the capability for them.
> > > > 
> > > > Once we know RC Core code has not been enabled, we can update
> > > > the user's capabilities and use them as a term of reference for
> > > > other RC-only calls.  This is preferable to having ugly #ifery
> > > > scattered throughout C code.
> > > > 
> > > > Most of the functions are actually safe to call, since they
> > > > sensibly check for a NULL RC pointer before they attempt to
> > > > deference it.
> > > > 
> > > > Signed-off-by: Lee Jones <lee.jo...@linaro.org>
> > > > ---
> > > >  drivers/media/cec/cec-core.c | 19 +++
> > > >  1 file changed, 7 insertions(+), 12 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
> > > > index cfe414a..51be8d6 100644
> > > > --- a/drivers/media/cec/cec-core.c
> > > > +++ b/drivers/media/cec/cec-core.c
> > > > @@ -208,9 +208,13 @@ struct cec_adapter *cec_allocate_adapter(const 
> > > > struct cec_adap_ops *ops,
> > > > return ERR_PTR(-EINVAL);
> > > > if (WARN_ON(!available_las || available_las > 
> > > > CEC_MAX_LOG_ADDRS))
> > > > return ERR_PTR(-EINVAL);
> > > > +   if (WARN_ON(caps & CEC_CAP_RC && !IS_REACHABLE(CONFIG_RC_CORE)))
> > > > +   caps &= ~CEC_CAP_RC;
> > > 
> > > Don't use WARN_ON, this is not an error of any kind.
> > 
> > Right, this is not an error.
> > 
> > That's why we are warning the user instead of bombing out.
> 
> Please print warning using pr_warn() or dev_warn().  Using WARN_ON()
> because something is not configured is _really_ not nice behaviour.
> Consider how useful a stack trace is to the user for this situation -
> it's completely meaningless.
> 
> A message that prompts the user to enable RC_CORE would make more sense,
> and be much more informative to the user.  Maybe something like this:
> 
> + if (caps & CEC_CAP_RC && !IS_REACHABLE(CONFIG_RC_CORE)) {
> + pr_warn("CEC: driver %pf requests RC, please enable 
> CONFIG_RC_CORE\n",
> +     __builtin_return_address(0));
> + caps &= ~CEC_CAP_RC;
> + }
> 
> It could be much more informative by using dev_warn() if we had the
> 'struct device' passed in to this function, and then we wouldn't need
> to use __builtin_return_address().

Understood.

I *would* fix, but Hans has made it pretty clear that this is not the
way he wants to go.  I still think a warning is the correct solution,
but for some reason we are to support out-of-tree drivers which might
be doing weird stuff.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog


Re: [PATCH] [media] cec: Handle RC capability more elegantly

2017-04-05 Thread Lee Jones
On Tue, 04 Apr 2017, Hans Verkuil wrote:

> On 04/04/2017 05:36 PM, Russell King - ARM Linux wrote:
> > On Tue, Apr 04, 2017 at 04:19:39PM +0100, Lee Jones wrote:
> >> On Tue, 04 Apr 2017, Hans Verkuil wrote:
> >>
> >>> On 04/04/2017 04:43 PM, Lee Jones wrote:
> >>>> If a user specifies the use of RC as a capability, they should
> >>>> really be enabling RC Core code.  If they do not we WARN() them
> >>>> of this and disable the capability for them.
> >>>>
> >>>> Once we know RC Core code has not been enabled, we can update
> >>>> the user's capabilities and use them as a term of reference for
> >>>> other RC-only calls.  This is preferable to having ugly #ifery
> >>>> scattered throughout C code.
> >>>>
> >>>> Most of the functions are actually safe to call, since they
> >>>> sensibly check for a NULL RC pointer before they attempt to
> >>>> deference it.
> >>>>
> >>>> Signed-off-by: Lee Jones <lee.jo...@linaro.org>
> >>>> ---
> >>>>  drivers/media/cec/cec-core.c | 19 +++
> >>>>  1 file changed, 7 insertions(+), 12 deletions(-)
> >>>>
> >>>> diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
> >>>> index cfe414a..51be8d6 100644
> >>>> --- a/drivers/media/cec/cec-core.c
> >>>> +++ b/drivers/media/cec/cec-core.c
> >>>> @@ -208,9 +208,13 @@ struct cec_adapter *cec_allocate_adapter(const 
> >>>> struct cec_adap_ops *ops,
> >>>>  return ERR_PTR(-EINVAL);
> >>>>  if (WARN_ON(!available_las || available_las > 
> >>>> CEC_MAX_LOG_ADDRS))
> >>>>  return ERR_PTR(-EINVAL);
> >>>> +if (WARN_ON(caps & CEC_CAP_RC && !IS_REACHABLE(CONFIG_RC_CORE)))
> >>>> +caps &= ~CEC_CAP_RC;
> >>>
> >>> Don't use WARN_ON, this is not an error of any kind.
> >>
> >> Right, this is not an error.
> >>
> >> That's why we are warning the user instead of bombing out.
> > 
> > Please print warning using pr_warn() or dev_warn().  Using WARN_ON()
> > because something is not configured is _really_ not nice behaviour.
> > Consider how useful a stack trace is to the user for this situation -
> > it's completely meaningless.
> > 
> > A message that prompts the user to enable RC_CORE would make more sense,
> > and be much more informative to the user.  Maybe something like this:
> > 
> > +   if (caps & CEC_CAP_RC && !IS_REACHABLE(CONFIG_RC_CORE)) {
> > +   pr_warn("CEC: driver %pf requests RC, please enable 
> > CONFIG_RC_CORE\n",
> > +   __builtin_return_address(0));
> > +   caps &= ~CEC_CAP_RC;
> > +   }
> > 
> > It could be much more informative by using dev_warn() if we had the
> > 'struct device' passed in to this function, and then we wouldn't need
> > to use __builtin_return_address().
> > 
> 
> I don't want to see a message logged because of this. In the current design it
> is perfectly valid to compile without RC_CORE.
> 
> I think eventually this should be redesigned a bit (a separate CEC config 
> option
> that enables or disables RC support), but for now I prefer to leave this as-is
> until I have a bit more experience with this.
> 
> After the CEC notifier work is in I will take another look at this.

Well at least I bought it to your attention.  I guess that's a 50% win.

I'll rework the patch accordingly.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog


[PATCH 1/2] [media] rc-core: Add inlined stubs for core rc_* functions

2017-04-04 Thread Lee Jones
Currently users have to use all sorts of ugly #ifery within
their drivers in order to avoid linking issues at build time.
This patch allows users to safely call these functions when
!CONFIG_RC_CORE and make decisions based on the return value
instead.  This is a much more common and clean way of doing
things within the Linux kernel.

Signed-off-by: Lee Jones <lee.jo...@linaro.org>
---
 include/media/rc-core.h | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 73ddd721..1f2043d 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -209,7 +209,14 @@ struct rc_dev {
  * @rc_driver_type: specifies the type of the RC output to be allocated
  * returns a pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 struct rc_dev *rc_allocate_device(enum rc_driver_type);
+#else
+static inline struct rc_dev *rc_allocate_device(int unused)
+{
+   return ERR_PTR(-EOPNOTSUPP);
+}
+#endif
 
 /**
  * devm_rc_allocate_device - Managed RC device allocation
@@ -218,21 +225,42 @@ struct rc_dev *rc_allocate_device(enum rc_driver_type);
  * @rc_driver_type: specifies the type of the RC output to be allocated
  * returns a pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 struct rc_dev *devm_rc_allocate_device(struct device *dev, enum 
rc_driver_type);
+#else
+static inline struct rc_dev *devm_rc_allocate_device(struct device *dev, int 
unused)
+{
+   return ERR_PTR(-EOPNOTSUPP);
+}
+#endif
 
 /**
  * rc_free_device - Frees a RC device
  *
  * @dev: pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 void rc_free_device(struct rc_dev *dev);
+#else
+static inline void rc_free_device(struct rc_dev *dev)
+{
+   return;
+}
+#endif
 
 /**
  * rc_register_device - Registers a RC device
  *
  * @dev: pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 int rc_register_device(struct rc_dev *dev);
+#else
+static inline int rc_register_device(struct rc_dev *dev)
+{
+   return -EOPNOTSUPP;
+}
+#endif
 
 /**
  * devm_rc_register_device - Manageded registering of a RC device
@@ -240,14 +268,28 @@ int rc_register_device(struct rc_dev *dev);
  * @parent: pointer to struct device.
  * @dev: pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 int devm_rc_register_device(struct device *parent, struct rc_dev *dev);
+#else
+static inline int devm_rc_register_device(struct device *parent, struct rc_dev 
*dev)
+{
+   return -EOPNOTSUPP;
+}
+#endif
 
 /**
  * rc_unregister_device - Unregisters a RC device
  *
  * @dev: pointer to struct rc_dev.
  */
+#ifdef CONFIG_RC_CORE
 void rc_unregister_device(struct rc_dev *dev);
+#else
+static inline void rc_unregister_device(struct rc_dev *dev)
+{
+   return;
+}
+#endif
 
 /**
  * rc_open - Opens a RC device
-- 
2.9.3



[PATCH 2/2] [media] cec: Handle RC capability more elegantly

2017-04-04 Thread Lee Jones
If a user specifies the use of RC as a capability, they should
really be enabling RC Core code.  If they do not we WARN() them
of this and disable the capability for them.

Once we know RC Core code has not been enabled, we can update
the user's capabilities and use them as a term of reference for
other RC-only calls.  This is preferable to having ugly #ifery
scattered throughout C code.

Most of the functions are actually safe to call, since they
sensibly check for a NULL RC pointer before they attempt to
deference it.

Signed-off-by: Lee Jones <lee.jo...@linaro.org>
---
 drivers/media/cec/cec-core.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index cfe414a..51be8d6 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -208,9 +208,13 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
return ERR_PTR(-EINVAL);
if (WARN_ON(!available_las || available_las > CEC_MAX_LOG_ADDRS))
return ERR_PTR(-EINVAL);
+   if (WARN_ON(caps & CEC_CAP_RC && !IS_REACHABLE(CONFIG_RC_CORE)))
+   caps &= ~CEC_CAP_RC;
+
adap = kzalloc(sizeof(*adap), GFP_KERNEL);
if (!adap)
return ERR_PTR(-ENOMEM);
+
strlcpy(adap->name, name, sizeof(adap->name));
adap->phys_addr = CEC_PHYS_ADDR_INVALID;
adap->log_addrs.cec_version = CEC_OP_CEC_VERSION_2_0;
@@ -237,7 +241,6 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
if (!(caps & CEC_CAP_RC))
return adap;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Prepare the RC input device */
adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
if (!adap->rc) {
@@ -264,9 +267,7 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
adap->rc->priv = adap;
adap->rc->map_name = RC_MAP_CEC;
adap->rc->timeout = MS_TO_NS(100);
-#else
-   adap->capabilities &= ~CEC_CAP_RC;
-#endif
+
return adap;
 }
 EXPORT_SYMBOL_GPL(cec_allocate_adapter);
@@ -285,7 +286,6 @@ int cec_register_adapter(struct cec_adapter *adap,
adap->owner = parent->driver->owner;
adap->devnode.dev.parent = parent;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
if (adap->capabilities & CEC_CAP_RC) {
adap->rc->dev.parent = parent;
res = rc_register_device(adap->rc);
@@ -298,15 +298,13 @@ int cec_register_adapter(struct cec_adapter *adap,
return res;
}
}
-#endif
 
res = cec_devnode_register(>devnode, adap->owner);
if (res) {
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Note: rc_unregister also calls rc_free */
rc_unregister_device(adap->rc);
adap->rc = NULL;
-#endif
+
return res;
}
 
@@ -337,11 +335,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
if (IS_ERR_OR_NULL(adap))
return;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Note: rc_unregister also calls rc_free */
rc_unregister_device(adap->rc);
adap->rc = NULL;
-#endif
+
debugfs_remove_recursive(adap->cec_dir);
cec_devnode_unregister(>devnode);
 }
@@ -357,9 +354,7 @@ void cec_delete_adapter(struct cec_adapter *adap)
kthread_stop(adap->kthread);
if (adap->kthread_config)
kthread_stop(adap->kthread_config);
-#if IS_REACHABLE(CONFIG_RC_CORE)
rc_free_device(adap->rc);
-#endif
kfree(adap);
 }
 EXPORT_SYMBOL_GPL(cec_delete_adapter);
-- 
2.9.3



Re: [PATCH] [media] cec: Handle RC capability more elegantly

2017-04-04 Thread Lee Jones
On Tue, 04 Apr 2017, Hans Verkuil wrote:

> On 04/04/2017 04:43 PM, Lee Jones wrote:
> > If a user specifies the use of RC as a capability, they should
> > really be enabling RC Core code.  If they do not we WARN() them
> > of this and disable the capability for them.
> > 
> > Once we know RC Core code has not been enabled, we can update
> > the user's capabilities and use them as a term of reference for
> > other RC-only calls.  This is preferable to having ugly #ifery
> > scattered throughout C code.
> > 
> > Most of the functions are actually safe to call, since they
> > sensibly check for a NULL RC pointer before they attempt to
> > deference it.
> > 
> > Signed-off-by: Lee Jones <lee.jo...@linaro.org>
> > ---
> >  drivers/media/cec/cec-core.c | 19 +++
> >  1 file changed, 7 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
> > index cfe414a..51be8d6 100644
> > --- a/drivers/media/cec/cec-core.c
> > +++ b/drivers/media/cec/cec-core.c
> > @@ -208,9 +208,13 @@ struct cec_adapter *cec_allocate_adapter(const struct 
> > cec_adap_ops *ops,
> > return ERR_PTR(-EINVAL);
> > if (WARN_ON(!available_las || available_las > CEC_MAX_LOG_ADDRS))
> > return ERR_PTR(-EINVAL);
> > +   if (WARN_ON(caps & CEC_CAP_RC && !IS_REACHABLE(CONFIG_RC_CORE)))
> > +   caps &= ~CEC_CAP_RC;
> 
> Don't use WARN_ON, this is not an error of any kind.

Right, this is not an error.

That's why we are warning the user instead of bombing out.

> Neither do you need to add the
> 'caps & CEC_CAP_RC' test. Really, it's just simpler to do what I suggested 
> before
> with an #if.

This does exactly what you asked.

Just to clarify, can you explain to me when asking for RC support, but
not enabling it would ever be a valid configuration?

> > +
> > adap = kzalloc(sizeof(*adap), GFP_KERNEL);
> > if (!adap)
> > return ERR_PTR(-ENOMEM);
> > +
> > strlcpy(adap->name, name, sizeof(adap->name));
> > adap->phys_addr = CEC_PHYS_ADDR_INVALID;
> > adap->log_addrs.cec_version = CEC_OP_CEC_VERSION_2_0;
> > @@ -237,7 +241,6 @@ struct cec_adapter *cec_allocate_adapter(const struct 
> > cec_adap_ops *ops,
> > if (!(caps & CEC_CAP_RC))
> > return adap;
> >  
> > -#if IS_REACHABLE(CONFIG_RC_CORE)
> 
> Huh? If CONFIG_RC_CORE is undefined, all these rc_ calls will fail when 
> linking!

I thought I'd tested for that, but it turns out that *my*
CONFIG_RC_CORE=n config was being over-ridden by the build system.

If it will really fail when linking, it sounds like the RC subsystem
is not written properly.  I guess that explains why all these drivers
are riddled with ugly #ifery.

Will fix that too, bear with.

> > /* Prepare the RC input device */
> > adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> > if (!adap->rc) {
> > @@ -264,9 +267,7 @@ struct cec_adapter *cec_allocate_adapter(const struct 
> > cec_adap_ops *ops,
> > adap->rc->priv = adap;
> > adap->rc->map_name = RC_MAP_CEC;
> > adap->rc->timeout = MS_TO_NS(100);
> > -#else
> > -   adap->capabilities &= ~CEC_CAP_RC;
> > -#endif
> > +
> > return adap;
> >  }
> >  EXPORT_SYMBOL_GPL(cec_allocate_adapter);
> > @@ -285,7 +286,6 @@ int cec_register_adapter(struct cec_adapter *adap,
> > adap->owner = parent->driver->owner;
> > adap->devnode.dev.parent = parent;
> >  
> > -#if IS_REACHABLE(CONFIG_RC_CORE)
> > if (adap->capabilities & CEC_CAP_RC) {
> > adap->rc->dev.parent = parent;
> > res = rc_register_device(adap->rc);
> > @@ -298,15 +298,13 @@ int cec_register_adapter(struct cec_adapter *adap,
> > return res;
> > }
> > }
> > -#endif
> >  
> > res = cec_devnode_register(>devnode, adap->owner);
> > if (res) {
> > -#if IS_REACHABLE(CONFIG_RC_CORE)
> > /* Note: rc_unregister also calls rc_free */
> > rc_unregister_device(adap->rc);
> > adap->rc = NULL;
> > -#endif
> > +
> > return res;
> > }
> >  
> > @@ -337,11 +335,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
> > if (IS_ERR_OR_NULL(adap))
> >         return;
> >  
> > -#if IS_REACHABLE(CONFIG_RC_CORE)
> > /* Note: rc_unregister also calls rc_free */
&

[PATCH] [media] cec: Handle RC capability more elegantly

2017-04-04 Thread Lee Jones
If a user specifies the use of RC as a capability, they should
really be enabling RC Core code.  If they do not we WARN() them
of this and disable the capability for them.

Once we know RC Core code has not been enabled, we can update
the user's capabilities and use them as a term of reference for
other RC-only calls.  This is preferable to having ugly #ifery
scattered throughout C code.

Most of the functions are actually safe to call, since they
sensibly check for a NULL RC pointer before they attempt to
deference it.

Signed-off-by: Lee Jones <lee.jo...@linaro.org>
---
 drivers/media/cec/cec-core.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index cfe414a..51be8d6 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -208,9 +208,13 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
return ERR_PTR(-EINVAL);
if (WARN_ON(!available_las || available_las > CEC_MAX_LOG_ADDRS))
return ERR_PTR(-EINVAL);
+   if (WARN_ON(caps & CEC_CAP_RC && !IS_REACHABLE(CONFIG_RC_CORE)))
+   caps &= ~CEC_CAP_RC;
+
adap = kzalloc(sizeof(*adap), GFP_KERNEL);
if (!adap)
return ERR_PTR(-ENOMEM);
+
strlcpy(adap->name, name, sizeof(adap->name));
adap->phys_addr = CEC_PHYS_ADDR_INVALID;
adap->log_addrs.cec_version = CEC_OP_CEC_VERSION_2_0;
@@ -237,7 +241,6 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
if (!(caps & CEC_CAP_RC))
return adap;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Prepare the RC input device */
adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
if (!adap->rc) {
@@ -264,9 +267,7 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
adap->rc->priv = adap;
adap->rc->map_name = RC_MAP_CEC;
adap->rc->timeout = MS_TO_NS(100);
-#else
-   adap->capabilities &= ~CEC_CAP_RC;
-#endif
+
return adap;
 }
 EXPORT_SYMBOL_GPL(cec_allocate_adapter);
@@ -285,7 +286,6 @@ int cec_register_adapter(struct cec_adapter *adap,
adap->owner = parent->driver->owner;
adap->devnode.dev.parent = parent;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
if (adap->capabilities & CEC_CAP_RC) {
adap->rc->dev.parent = parent;
res = rc_register_device(adap->rc);
@@ -298,15 +298,13 @@ int cec_register_adapter(struct cec_adapter *adap,
return res;
}
}
-#endif
 
res = cec_devnode_register(>devnode, adap->owner);
if (res) {
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Note: rc_unregister also calls rc_free */
rc_unregister_device(adap->rc);
adap->rc = NULL;
-#endif
+
return res;
}
 
@@ -337,11 +335,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
if (IS_ERR_OR_NULL(adap))
return;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Note: rc_unregister also calls rc_free */
rc_unregister_device(adap->rc);
adap->rc = NULL;
-#endif
+
debugfs_remove_recursive(adap->cec_dir);
cec_devnode_unregister(>devnode);
 }
@@ -357,9 +354,7 @@ void cec_delete_adapter(struct cec_adapter *adap)
kthread_stop(adap->kthread);
if (adap->kthread_config)
kthread_stop(adap->kthread_config);
-#if IS_REACHABLE(CONFIG_RC_CORE)
rc_free_device(adap->rc);
-#endif
kfree(adap);
 }
 EXPORT_SYMBOL_GPL(cec_delete_adapter);
-- 
2.9.3



Re: [PATCH 1/2] [media] cec: Move capability check inside #if

2017-04-04 Thread Lee Jones
On Tue, 04 Apr 2017, Hans Verkuil wrote:

> On 04/04/2017 03:01 PM, Lee Jones wrote:
> > On Tue, 04 Apr 2017, Lee Jones wrote:
> > 
> >> On Tue, 04 Apr 2017, Hans Verkuil wrote:
> >>
> >>> On 04/04/2017 02:32 PM, Lee Jones wrote:
> >>>> If CONFIG_RC_CORE is not enabled then none of the RC code will be
> >>>> executed anyway, so we're placing the capability check inside the
> >>>>
> >>>> Signed-off-by: Lee Jones <lee.jo...@linaro.org>
> >>>> ---
> >>>>  drivers/media/cec/cec-core.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
> >>>> index 37217e2..06a312c 100644
> >>>> --- a/drivers/media/cec/cec-core.c
> >>>> +++ b/drivers/media/cec/cec-core.c
> >>>> @@ -234,10 +234,10 @@ struct cec_adapter *cec_allocate_adapter(const 
> >>>> struct cec_adap_ops *ops,
> >>>>  return ERR_PTR(res);
> >>>>  }
> >>>>  
> >>>> +#if IS_REACHABLE(CONFIG_RC_CORE)
> >>>>  if (!(caps & CEC_CAP_RC))
> >>>>  return adap;
> >>>>  
> >>>> -#if IS_REACHABLE(CONFIG_RC_CORE)
> >>>>  /* Prepare the RC input device */
> >>>>  adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> >>>>  if (!adap->rc) {
> >>>>
> >>>
> >>> Not true, there is an #else further down.
> >>
> >> I saw the #else.  It's inert code that becomes function-less.
> >>
> >>> That said, this code is clearly a bit confusing.
> >>>
> >>> It would be better if at the beginning of the function we'd have this:
> >>>
> >>> #if !IS_REACHABLE(CONFIG_RC_CORE)
> >>>   caps &= ~CEC_CAP_RC;
> >>> #endif
> >>>
> >>> and then drop the #else bit and (as you do in this patch) move the #if up.
> >>>
> >>> Can you make a new patch for this?
> >>
> >> Sure.
> > 
> > No wait, sorry!  This patch is the correct fix.
> > 
> > 'caps' is already indicating !CEC_CAP_RC, which is right.
> > 
> > What we're trying to do here is only consider looking at the
> > capabilities if the RC Core is enabled.  If it is not enabled, the #if
> > still does the right thing and makes sure that the caps are updated.
> > 
> > Please take another look at the semantics.
> 
> Ah, yes. You are right. But so am I: the code is just unnecessarily confusing
> as is seen by this discussion.
> 
> I still would like to see a patch with my proposed solution. The control flow
> is much easier to understand that way.

I have an idea.  Please bear with me.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog


Re: [PATCH 1/2] [media] cec: Move capability check inside #if

2017-04-04 Thread Lee Jones
On Tue, 04 Apr 2017, Lee Jones wrote:

> On Tue, 04 Apr 2017, Hans Verkuil wrote:
> 
> > On 04/04/2017 02:32 PM, Lee Jones wrote:
> > > If CONFIG_RC_CORE is not enabled then none of the RC code will be
> > > executed anyway, so we're placing the capability check inside the
> > > 
> > > Signed-off-by: Lee Jones <lee.jo...@linaro.org>
> > > ---
> > >  drivers/media/cec/cec-core.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
> > > index 37217e2..06a312c 100644
> > > --- a/drivers/media/cec/cec-core.c
> > > +++ b/drivers/media/cec/cec-core.c
> > > @@ -234,10 +234,10 @@ struct cec_adapter *cec_allocate_adapter(const 
> > > struct cec_adap_ops *ops,
> > >   return ERR_PTR(res);
> > >   }
> > >  
> > > +#if IS_REACHABLE(CONFIG_RC_CORE)
> > >   if (!(caps & CEC_CAP_RC))
> > >   return adap;
> > >  
> > > -#if IS_REACHABLE(CONFIG_RC_CORE)
> > >   /* Prepare the RC input device */
> > >   adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> > >   if (!adap->rc) {
> > > 
> > 
> > Not true, there is an #else further down.
> 
> I saw the #else.  It's inert code that becomes function-less.
> 
> > That said, this code is clearly a bit confusing.
> > 
> > It would be better if at the beginning of the function we'd have this:
> > 
> > #if !IS_REACHABLE(CONFIG_RC_CORE)
> > caps &= ~CEC_CAP_RC;
> > #endif
> > 
> > and then drop the #else bit and (as you do in this patch) move the #if up.
> > 
> > Can you make a new patch for this?
> 
> Sure.

No wait, sorry!  This patch is the correct fix.

'caps' is already indicating !CEC_CAP_RC, which is right.

What we're trying to do here is only consider looking at the
capabilities if the RC Core is enabled.  If it is not enabled, the #if
still does the right thing and makes sure that the caps are updated.

Please take another look at the semantics.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog


Re: [PATCH 2/2] [media] cec: Fix runtime BUG when (CONFIG_RC_CORE && !CEC_CAP_RC)

2017-04-04 Thread Lee Jones
On Tue, 04 Apr 2017, Hans Verkuil wrote:

> On 04/04/2017 02:32 PM, Lee Jones wrote:
> > Currently when the RC Core is enabled (reachable) core code located
> > in cec_register_adapter() attempts to populate the RC structure with
> > a pointer to the 'parent' passed in by the caller.
> > 
> > Unfortunately if the caller did not specify RC capibility when calling
> > cec_allocate_adapter(), then there will be no RC structure to populate.
> > 
> > This causes a "NULL pointer dereference" error.
> > 
> > Fixes: f51e80804f0 ("[media] cec: pass parent device in register(), not 
> > allocate()")
> > Signed-off-by: Lee Jones <lee.jo...@linaro.org>
> 
> Oops! Thanks for the report. I'll take this for 4.12.

Since this is a -fix, it should really go in for v4.11.

> > ---
> >  drivers/media/cec/cec-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
> > index 06a312c..d64937b 100644
> > --- a/drivers/media/cec/cec-core.c
> > +++ b/drivers/media/cec/cec-core.c
> > @@ -286,8 +286,8 @@ int cec_register_adapter(struct cec_adapter *adap,
> > adap->devnode.dev.parent = parent;
> >  
> >  #if IS_REACHABLE(CONFIG_RC_CORE)
> > -   adap->rc->dev.parent = parent;
> > if (adap->capabilities & CEC_CAP_RC) {
> > +   adap->rc->dev.parent = parent;
> > res = rc_register_device(adap->rc);
> >  
> > if (res) {
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog


Re: [PATCH 1/2] [media] cec: Move capability check inside #if

2017-04-04 Thread Lee Jones
On Tue, 04 Apr 2017, Hans Verkuil wrote:

> On 04/04/2017 02:32 PM, Lee Jones wrote:
> > If CONFIG_RC_CORE is not enabled then none of the RC code will be
> > executed anyway, so we're placing the capability check inside the
> > 
> > Signed-off-by: Lee Jones <lee.jo...@linaro.org>
> > ---
> >  drivers/media/cec/cec-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
> > index 37217e2..06a312c 100644
> > --- a/drivers/media/cec/cec-core.c
> > +++ b/drivers/media/cec/cec-core.c
> > @@ -234,10 +234,10 @@ struct cec_adapter *cec_allocate_adapter(const struct 
> > cec_adap_ops *ops,
> > return ERR_PTR(res);
> > }
> >  
> > +#if IS_REACHABLE(CONFIG_RC_CORE)
> > if (!(caps & CEC_CAP_RC))
> > return adap;
> >  
> > -#if IS_REACHABLE(CONFIG_RC_CORE)
> > /* Prepare the RC input device */
> > adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> > if (!adap->rc) {
> > 
> 
> Not true, there is an #else further down.

I saw the #else.  It's inert code that becomes function-less.

> That said, this code is clearly a bit confusing.
> 
> It would be better if at the beginning of the function we'd have this:
> 
> #if !IS_REACHABLE(CONFIG_RC_CORE)
>   caps &= ~CEC_CAP_RC;
> #endif
> 
> and then drop the #else bit and (as you do in this patch) move the #if up.
> 
> Can you make a new patch for this?

Sure.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog


[PATCH 2/2] [media] cec: Fix runtime BUG when (CONFIG_RC_CORE && !CEC_CAP_RC)

2017-04-04 Thread Lee Jones
Currently when the RC Core is enabled (reachable) core code located
in cec_register_adapter() attempts to populate the RC structure with
a pointer to the 'parent' passed in by the caller.

Unfortunately if the caller did not specify RC capibility when calling
cec_allocate_adapter(), then there will be no RC structure to populate.

This causes a "NULL pointer dereference" error.

Fixes: f51e80804f0 ("[media] cec: pass parent device in register(), not 
allocate()")
Signed-off-by: Lee Jones <lee.jo...@linaro.org>
---
 drivers/media/cec/cec-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index 06a312c..d64937b 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -286,8 +286,8 @@ int cec_register_adapter(struct cec_adapter *adap,
adap->devnode.dev.parent = parent;
 
 #if IS_REACHABLE(CONFIG_RC_CORE)
-   adap->rc->dev.parent = parent;
if (adap->capabilities & CEC_CAP_RC) {
+   adap->rc->dev.parent = parent;
res = rc_register_device(adap->rc);
 
if (res) {
-- 
2.9.3



[PATCH 1/2] [media] cec: Move capability check inside #if

2017-04-04 Thread Lee Jones
If CONFIG_RC_CORE is not enabled then none of the RC code will be
executed anyway, so we're placing the capability check inside the

Signed-off-by: Lee Jones <lee.jo...@linaro.org>
---
 drivers/media/cec/cec-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index 37217e2..06a312c 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -234,10 +234,10 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
return ERR_PTR(res);
}
 
+#if IS_REACHABLE(CONFIG_RC_CORE)
if (!(caps & CEC_CAP_RC))
return adap;
 
-#if IS_REACHABLE(CONFIG_RC_CORE)
/* Prepare the RC input device */
adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
if (!adap->rc) {
-- 
2.9.3



Re: [PATCH v2 3/3] [media] include/media: move platform_data to linux/platform_data/media

2015-11-16 Thread Lee Jones
On Mon, 16 Nov 2015, Mauro Carvalho Chehab wrote:

> Let's not mix platform_data headers with the core headers. Instead, let's
> create a subdir at linux/platform_data and move the headers to that
> common place, adding it to MAINTAINERS.
> 
> The headers were moved with:
>   mkdir include/linux/platform_data/media/; git mv 
> include/media/gpio-ir-recv.h include/media/ir-rx51.h 
> include/media/mmp-camera.h include/media/omap1_camera.h 
> include/media/omap4iss.h include/media/s5p_hdmi.h include/media/si4713.h 
> include/media/sii9234.h include/media/smiapp.h include/media/soc_camera.h 
> include/media/soc_camera_platform.h include/media/timb_radio.h 
> include/media/timb_video.h include/linux/platform_data/media/
> 
> And the references fixed with this script:
> MAIN_DIR="linux/platform_data/"
> PREV_DIR="media/"
> DIRS="media/"
> 
> echo "Checking affected files" >&2
> for i in $DIRS; do
>   for j in $(find include/$MAIN_DIR/$i -type f -name '*.h'); do
>n=`basename $j`
>   git grep -l $n
>   done
> done|sort|uniq >files && (
>   echo "Handling files..." >&2;
>   echo "for i in \$(cat files|grep -v Documentation); do cat \$i | \\";
>   (
>   cd include/$MAIN_DIR;
>   for j in $DIRS; do
>   for i in $(ls $j); do
>   echo "perl -ne 's,(include 
> [\\\"\\<])$PREV_DIR($i)([\\\"\\>]),\1$MAIN_DIR$j\2\3,; print \$_' |\\";
>   done;
>   done;
>   echo "cat > a && mv a \$i; done";
>   );
>   echo "Handling documentation..." >&2;
>   echo "for i in MAINTAINERS \$(cat files); do cat \$i | \\";
>   (
>   cd include/$MAIN_DIR;
>   for j in $DIRS; do
>   for i in $(ls $j); do
>   echo "  perl -ne 
> 's,include/$PREV_DIR($i)\b,include/$MAIN_DIR$j\1,; print \$_' |\\";
>   done;
>   done;
>   echo "cat > a && mv a \$i; done"
>   );
> ) >script && . ./script
> 
> Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
> ---
>  Documentation/video4linux/omap4_camera.txt| 2 +-
>  Documentation/video4linux/si4713.txt  | 2 +-
>  MAINTAINERS   | 1 +
>  arch/arm/mach-omap1/include/mach/camera.h | 2 +-
>  arch/arm/mach-omap2/board-rx51-peripherals.c  | 4 ++--
>  arch/arm/plat-samsung/devs.c  | 2 +-
>  arch/sh/boards/mach-ap325rxa/setup.c  | 2 +-
>  drivers/media/platform/marvell-ccic/mmp-driver.c  | 2 +-
>  drivers/media/platform/s5p-tv/hdmi_drv.c  | 2 +-
>  drivers/media/platform/s5p-tv/sii9234_drv.c   | 2 +-
>  drivers/media/platform/soc_camera/omap1_camera.c  | 2 +-
>  drivers/media/platform/soc_camera/soc_camera_platform.c   | 2 +-
>  drivers/media/platform/timblogiw.c| 2 +-
>  drivers/media/radio/radio-timb.c          | 2 +-
>  drivers/media/radio/si4713/radio-usb-si4713.c | 2 +-
>  drivers/media/radio/si4713/si4713.h   | 2 +-
>  drivers/media/rc/gpio-ir-recv.c   | 2 +-
>  drivers/media/rc/ir-rx51.c| 2 +-
>  drivers/mfd/timberdale.c  | 4 ++--

Acked-by: Lee Jones <lee.jo...@linaro.org>
  
>  drivers/staging/media/omap4iss/iss.h  | 2 +-
>  drivers/staging/media/omap4iss/iss_csiphy.h   | 2 +-
>  include/{ => linux/platform_data}/media/gpio-ir-recv.h| 1 -
>  include/{ => linux/platform_data}/media/ir-rx51.h | 0
>  include/{ => linux/platform_data}/media/mmp-camera.h  | 0
>  include/{ => linux/platform_data}/media/omap1_camera.h| 0
>  include/{ => linux/platform_data}/media/omap4iss.h| 0
>  include/{ => linux/platform_data}/media/s5p_hdmi.h| 1 -
>  include/{ => linux/platform_data}/media/si4713.h  | 2 +-
>  include/{ => linux/platform_data}/media/sii9234.h | 0
>  include/{ => linux/platform_data}/media/soc_camera_platform.h | 0
>  include/{ => linux/platform_data}/media/timb_radio.h  | 0
>  include/{ => linux/platform_data}/media/timb_video.h

Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios

2015-09-01 Thread Lee Jones
On Mon, 31 Aug 2015, Rob Herring wrote:

> On Fri, Aug 28, 2015 at 12:52 PM, Peter Griffin
> <peter.grif...@linaro.org> wrote:
> > gpio.txt documents that GPIO properties should be named
> > "[-]gpios", with  being the purpose of this
> > GPIO for the device.
> >
> > This change has been done as one atomic commit.
> 
> dtb and kernel updates are not necessarily atomic, so you are breaking
> compatibility with older dtbs. You should certainly highlight that in
> the commit message.

Good idea.

> I only point this out. I'll leave it to platform
> maintainers whether or not this breakage is acceptable.

This driver is new.  The 'original' bindings are in next.  So this
binding is not even close to being ABI.

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-media" 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 4/6] [media] c8sectpfe: Update binding to reset-gpios

2015-09-01 Thread Lee Jones
On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.grif...@linaro.org> 
> wrote:
> > gpio.txt documents that GPIO properties should be named
> > "[-]gpios", with  being the purpose of this
> > GPIO for the device.
> >
> > This change has been done as one atomic commit.
> >
> > Signed-off-by: Peter Griffin <peter.grif...@linaro.org>
> > Acked-by: Lee Jones <lee.jo...@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
> >  arch/arm/boot/dts/stihxxx-b2120.dtsi  | 4 ++--
> >  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 2 +-
> >  3 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt 
> > b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > index d4def76..e70d840 100644
> > --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
> >
> >  - tsin-num : tsin id of the InputBlock (must be between 0 to 6)
> >  - i2c-bus  : phandle to the I2C bus DT node which the demodulators & 
> > tuners on this tsin channel are connected.
> > -- rst-gpio : reset gpio for this tsin channel.
> > +- reset-gpios  : reset gpio for this tsin channel.
> 
> The documentation is a bit outdated, the GPIO subsystem supports both
> -gpio and -gpios, see commit:
> 
> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
> 
> So it makes sense to me to use -gpio instead of -gpios in this case
> since is a single GPIO. Also rst is already a descriptive name since
> that's how many datasheets name a reset pin. I'm not saying I'm
> against this patch, just pointing out since the commit message is a
> bit misleading.

As I suggested this patch, I feel I must comment.

My order of preference would be:

 reset-gpio
 reset-gpios
 rst-gpio
 rst-gpios

This current patch is No2, so it's okay to stay IMHO.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-media" 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 2/6] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node.

2015-09-01 Thread Lee Jones
On Fri, 28 Aug 2015, Peter Griffin wrote:

> This patch adds in the required DT node for the c8sectpfe
> Linux DVB demux driver which allows the tsin channels
> to be used on an upstream kernel.
> 
> Signed-off-by: Peter Griffin <peter.grif...@linaro.org>
> ---
>  arch/arm/boot/dts/stihxxx-b2120.dtsi | 35 +++
>  1 file changed, 35 insertions(+)

Acked-by: Lee Jones <lee.jo...@linaro.org>

> diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi 
> b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> index 62994ae..f9fca10 100644
> --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
> +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> @@ -6,6 +6,9 @@
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
> +
> +#include 
> +#include 
>  / {
>   soc {
>   sbc_serial0: serial@953 {
> @@ -85,5 +88,37 @@
>   status = "okay";
>   };
>  
> + demux@08a2 {
> + compatible  = "st,stih407-c8sectpfe";
> + status  = "okay";
> + reg = <0x08a2 0x1>,
> +   <0x08a0 0x4000>;
> + reg-names   = "c8sectpfe", "c8sectpfe-ram";
> + interrupts  = ,
> +   ;
> + interrupt-names = "c8sectpfe-error-irq",
> +   "c8sectpfe-idle-irq";
> + pinctrl-0   = <_tsin0_serial>;
> + pinctrl-1   = <_tsin0_parallel>;
> + pinctrl-2   = <_tsin3_serial>;
> + pinctrl-3   = <_tsin4_serial_alt3>;
> + pinctrl-4   = <_tsin5_serial_alt1>;
> + pinctrl-names   = "tsin0-serial",
> +   "tsin0-parallel",
> +   "tsin3-serial",
> +   "tsin4-serial",
> +   "tsin5-serial";
> + clocks  = <_s_c0_flexgen CLK_PROC_STFE>;
> + clock-names = "c8sectpfe";
> +
> + /* tsin0 is TSA on NIMA */
> + tsin0: port@0 {
> +         tsin-num= <0>;
> + serial-not-parallel;
> + i2c-bus = <>;
> + rst-gpio= < 4 GPIO_ACTIVE_HIGH>;
> + dvb-card= ;
> + };
> + };
>   };
>  };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-media" 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 4/6] [media] c8sectpfe: Update binding to reset-gpios

2015-09-01 Thread Lee Jones
On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:

> Hello Lee,
> 
> On Tue, Sep 1, 2015 at 11:09 AM, Lee Jones <lee.jo...@linaro.org> wrote:
> > On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
> >> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.grif...@linaro.org> 
> >> wrote:
> >> > gpio.txt documents that GPIO properties should be named
> >> > "[-]gpios", with  being the purpose of this
> >> > GPIO for the device.
> >> >
> >> > This change has been done as one atomic commit.
> >> >
> >> > Signed-off-by: Peter Griffin <peter.grif...@linaro.org>
> >> > Acked-by: Lee Jones <lee.jo...@linaro.org>
> >> > ---
> >> >  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
> >> >  arch/arm/boot/dts/stihxxx-b2120.dtsi  | 4 ++--
> >> >  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 2 +-
> >> >  3 files changed, 6 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git 
> >> > a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt 
> >> > b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> > index d4def76..e70d840 100644
> >> > --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> > +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> > @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
> >> >
> >> >  - tsin-num : tsin id of the InputBlock (must be between 0 to 6)
> >> >  - i2c-bus  : phandle to the I2C bus DT node which the demodulators 
> >> > & tuners on this tsin channel are connected.
> >> > -- rst-gpio : reset gpio for this tsin channel.
> >> > +- reset-gpios  : reset gpio for this tsin channel.
> >>
> >> The documentation is a bit outdated, the GPIO subsystem supports both
> >> -gpio and -gpios, see commit:
> >>
> >> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
> >>
> >> So it makes sense to me to use -gpio instead of -gpios in this case
> >> since is a single GPIO. Also rst is already a descriptive name since
> >> that's how many datasheets name a reset pin. I'm not saying I'm
> >> against this patch, just pointing out since the commit message is a
> >> bit misleading.
> >
> > As I suggested this patch, I feel I must comment.
> >
> > My order of preference would be:
> >
> >  reset-gpio
> >  reset-gpios
> >  rst-gpio
> >  rst-gpios
> >
> > This current patch is No2, so it's okay to stay IMHO.
> >
> 
> If the property is being changed anyways, why not going with No1 then?
> 
> As I said, I'm not against the patch but I think the commit message
> has to be reworded since implies that the problem is that the -gpio
> sufix is being used instead of -gpios. But since both are supported by
> the GPIO subsystem, the commit should mention that "reset" is more
> clear and easier to read than "rst" or something along those lines.
> 
> BTW, I just posted a patch for the GPIO doc to mention that -gpio is
> also supported:
> 
> https://patchwork.kernel.org/patch/7103761/

Noted.  Thanks for the heads-up.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-media" 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 4/6] [media] c8sectpfe: Update binding to reset-gpios

2015-09-01 Thread Lee Jones
On Tue, 01 Sep 2015, Peter Griffin wrote:
> On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
> > On Tue, Sep 1, 2015 at 11:09 AM, Lee Jones <lee.jo...@linaro.org> wrote:
> > > On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
> > >> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin 
> > >> <peter.grif...@linaro.org> wrote:
> > >> > gpio.txt documents that GPIO properties should be named
> > >> > "[-]gpios", with  being the purpose of this
> > >> > GPIO for the device.
> > >> >
> > >> > This change has been done as one atomic commit.
> > >> >
> > >> > Signed-off-by: Peter Griffin <peter.grif...@linaro.org>
> > >> > Acked-by: Lee Jones <lee.jo...@linaro.org>
> > >> > ---
> > >> >  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 
> > >> > +++---
> > >> >  arch/arm/boot/dts/stihxxx-b2120.dtsi  | 4 ++--
> > >> >  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 2 +-
> > >> >  3 files changed, 6 insertions(+), 6 deletions(-)
> > >> >
> > >> > diff --git 
> > >> > a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt 
> > >> > b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > >> > index d4def76..e70d840 100644
> > >> > --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > >> > +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > >> > @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
> > >> >
> > >> >  - tsin-num : tsin id of the InputBlock (must be between 0 to 6)
> > >> >  - i2c-bus  : phandle to the I2C bus DT node which the 
> > >> > demodulators & tuners on this tsin channel are connected.
> > >> > -- rst-gpio : reset gpio for this tsin channel.
> > >> > +- reset-gpios  : reset gpio for this tsin channel.
> > >>
> > >> The documentation is a bit outdated, the GPIO subsystem supports both
> > >> -gpio and -gpios, see commit:
> > >>
> > >> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
> > >>
> > >> So it makes sense to me to use -gpio instead of -gpios in this case
> > >> since is a single GPIO. Also rst is already a descriptive name since
> > >> that's how many datasheets name a reset pin. I'm not saying I'm
> > >> against this patch, just pointing out since the commit message is a
> > >> bit misleading.
> 
> Ok thanks for pointing that out. It's nice to know the original binding was
> actually OK.

By luck, rather than judgement. ;)

> > > As I suggested this patch, I feel I must comment.
> > >
> > > My order of preference would be:
> > >
> > >  reset-gpio
> > >  reset-gpios
> > >  rst-gpio
> > >  rst-gpios
> > >
> > > This current patch is No2, so it's okay to stay IMHO.
> > >
> > 
> > If the property is being changed anyways, why not going with No1 then?
> 
> I've changed to No1 in v4.

Nice!  Thanks for fixing up.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-media" 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 4/5] [media] c8sectpfe: Update binding to reset-gpios

2015-08-28 Thread Lee Jones
On Thu, 27 Aug 2015, Peter Griffin wrote:

 gpio.txt documents that GPIO properties should be named
 [name-]gpios, with name being the purpose of this
 GPIO for the device.
 
 This change has been done as one atomic commit.
 
 Signed-off-by: Peter Griffin peter.grif...@linaro.org
 ---
  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
  arch/arm/boot/dts/stihxxx-b2120.dtsi  | 4 ++--
  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 2 +-
  3 files changed, 6 insertions(+), 6 deletions(-)

Acked-by: Lee Jones lee.jo...@linaro.org

 diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt 
 b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
 index d4def76..e70d840 100644
 --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
 +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
 @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
  
  - tsin-num   : tsin id of the InputBlock (must be between 0 to 6)
  - i2c-bus: phandle to the I2C bus DT node which the demodulators  
 tuners on this tsin channel are connected.
 -- rst-gpio   : reset gpio for this tsin channel.
 +- reset-gpios: reset gpio for this tsin channel.
  
  Optional properties (tsin (child) node):
  
 @@ -75,7 +75,7 @@ Example:
   tsin-num= 0;
   serial-not-parallel;
   i2c-bus = ssc2;
 - rst-gpio= pio15 4 0;
 + reset-gpios = pio15 4 GPIO_ACTIVE_HIGH;
   dvb-card= STV0367_TDA18212_NIMA_1;
   };
  
 @@ -83,7 +83,7 @@ Example:
   tsin-num= 3;
   serial-not-parallel;
   i2c-bus = ssc3;
 - rst-gpio= pio15 7 0;
 + reset-gpios = pio15 7 GPIO_ACTIVE_HIGH;
   dvb-card= STV0367_TDA18212_NIMB_1;
   };
   };
 diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi 
 b/arch/arm/boot/dts/stihxxx-b2120.dtsi
 index c014173..f932bfd 100644
 --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
 +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
 @@ -6,8 +6,8 @@
   * it under the terms of the GNU General Public License version 2 as
   * published by the Free Software Foundation.
   */
 -
  #include dt-bindings/clock/stih407-clks.h
 +#include dt-bindings/gpio/gpio.h
  #include dt-bindings/media/c8sectpfe.h
  / {
   soc {
 @@ -115,7 +115,7 @@
   tsin-num= 0;
   serial-not-parallel;
   i2c-bus = ssc2;
 - rst-gpio= pio15 4 0;
 + reset-gpios = pio15 4 GPIO_ACTIVE_HIGH;
   dvb-card= STV0367_TDA18212_NIMA_1;
   };
   };
 diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c 
 b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
 index 3a91093..c691e13 100644
 --- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
 +++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
 @@ -822,7 +822,7 @@ static int c8sectpfe_probe(struct platform_device *pdev)
   }
   of_node_put(i2c_bus);
  
 - tsin-rst_gpio = of_get_named_gpio(child, rst-gpio, 0);
 + tsin-rst_gpio = of_get_named_gpio(child, reset-gpios, 0);
  
   ret = gpio_is_valid(tsin-rst_gpio);
   if (!ret) {

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 5/5] [media] c8sectpfe: Update DT binding doc with some minor fixes

2015-08-28 Thread Lee Jones
On Thu, 27 Aug 2015, Peter Griffin wrote:

 Signed-off-by: Peter Griffin peter.grif...@linaro.org
 ---
  .../devicetree/bindings/media/stih407-c8sectpfe.txt  | 16 
 +++-
  1 file changed, 7 insertions(+), 9 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt 
 b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
 index e70d840..5d6438c 100644
 --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
 +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
 @@ -55,21 +55,20 @@ Example:
   status = okay;
   reg = 0x08a2 0x1, 0x08a0 0x4000;
   reg-names = stfe, stfe-ram;
 - interrupts = 0 34 0, 0 35 0;
 + interrupts = GIC_SPI 34 IRQ_TYPE_NONE, GIC_SPI 35 
 IRQ_TYPE_NONE;
   interrupt-names = stfe-error-irq, stfe-idle-irq;
 -
 - pinctrl-names   = tsin0-serial, tsin0-parallel, 
 tsin3-serial,
 - tsin4-serial, tsin5-serial;
 -
 + pinctrl-names   = tsin0-serial,
 +   tsin0-parallel,
 +   tsin3-serial,
 +   tsin4-serial,
 +   tsin5-serial;
   pinctrl-0   = pinctrl_tsin0_serial;
   pinctrl-1   = pinctrl_tsin0_parallel;
   pinctrl-2   = pinctrl_tsin3_serial;
   pinctrl-3   = pinctrl_tsin4_serial_alt3;
   pinctrl-4   = pinctrl_tsin5_serial_alt1;
 -
   clocks = clk_s_c0_flexgen CLK_PROC_STFE;
 - clock-names = stfe;
 -
 + clock-names = c8sectpfe;

Are you sure you even need this property?  I'm thinking that *-names
properties are *usually* only required if there are more than one in a
single property.

   /* tsin0 is TSA on NIMA */
   tsin0: port@0 {
   tsin-num= 0;
 @@ -78,7 +77,6 @@ Example:
   reset-gpios = pio15 4 GPIO_ACTIVE_HIGH;
   dvb-card= STV0367_TDA18212_NIMA_1;
   };
 -

My personal preference is to have a '\n' between nodes.

   tsin3: port@3 {
   tsin-num= 3;
   serial-not-parallel;

But these comments are pretty trivial, so agree or not, or fix-up or
not, it's that big of a deal.

Either way,
  Acked-by: Lee Jones lee.jo...@linaro.org

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 3/5] [media] c8sectpfe: Remove select on undefined LIBELF_32

2015-08-28 Thread Lee Jones
On Thu, 27 Aug 2015, Peter Griffin wrote:

 LIBELF_32 is not defined in Kconfig, and is left over legacy
 which is not required in the upstream driver, so remove it.
 
 Signed-off-by: Peter Griffin peter.grif...@linaro.org
 Suggested-by: Valentin Rothberg valentinrothb...@gmail.com

These are the wrong way round, they should be in chronological order.

 ---
  drivers/media/platform/sti/c8sectpfe/Kconfig | 1 -
  1 file changed, 1 deletion(-)

Acked-by: Lee Jones lee.jo...@linaro.org

 diff --git a/drivers/media/platform/sti/c8sectpfe/Kconfig 
 b/drivers/media/platform/sti/c8sectpfe/Kconfig
 index d1bfd4c..b9ec667 100644
 --- a/drivers/media/platform/sti/c8sectpfe/Kconfig
 +++ b/drivers/media/platform/sti/c8sectpfe/Kconfig
 @@ -1,7 +1,6 @@
  config DVB_C8SECTPFE
   tristate STMicroelectronics C8SECTPFE DVB support
   depends on DVB_CORE  I2C  (ARCH_STI || ARCH_MULTIPLATFORM)
 - select LIBELF_32
   select FW_LOADER
   select FW_LOADER_USER_HELPER_FALLBACK
   select DEBUG_FS

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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/5] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node.

2015-08-28 Thread Lee Jones
On Thu, 27 Aug 2015, Peter Griffin wrote:

 This patch adds in the required DT node for the c8sectpfe
 Linux DVB demux driver which allows the tsin channels
 to be used on an upstream kernel.
 
 Signed-off-by: Peter Griffin peter.grif...@linaro.org
 ---
  arch/arm/boot/dts/stihxxx-b2120.dtsi | 34 ++
  1 file changed, 34 insertions(+)
 
 diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi 
 b/arch/arm/boot/dts/stihxxx-b2120.dtsi
 index 62994ae..c014173 100644
 --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
 +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
 @@ -6,6 +6,9 @@
   * it under the terms of the GNU General Public License version 2 as
   * published by the Free Software Foundation.
   */
 +
 +#include dt-bindings/clock/stih407-clks.h
 +#include dt-bindings/media/c8sectpfe.h
  / {
   soc {
   sbc_serial0: serial@953 {
 @@ -85,5 +88,36 @@
   status = okay;
   };
  
 + demux@08a2 {
 + compatible  = st,stih407-c8sectpfe;
 + status  = okay;
 + reg = 0x08a2 0x1,
 +   0x08a0 0x4000;

These look like they're the wrong way round.

 + reg-names   = c8sectpfe, c8sectpfe-ram;
 + interrupts  = GIC_SPI 34 IRQ_TYPE_NONE,
 +   GIC_SPI 35 IRQ_TYPE_NONE;
 + interrupt-names = c8sectpfe-error-irq,
 +   c8sectpfe-idle-irq;
 + pinctrl-names   = tsin0-serial,
 +   tsin0-parallel,
 +   tsin3-serial,
 +   tsin4-serial,
 +   tsin5-serial;
 + pinctrl-0   = pinctrl_tsin0_serial;
 + pinctrl-1   = pinctrl_tsin0_parallel;
 + pinctrl-2   = pinctrl_tsin3_serial;
 + pinctrl-3   = pinctrl_tsin4_serial_alt3;
 + pinctrl-4   = pinctrl_tsin5_serial_alt1;
 + clock-names = c8sectpfe;
 + clocks  = clk_s_c0_flexgen CLK_PROC_STFE;

Personal preferenc is that the *-names properties should come *after*
the ones they reference.

 + /* tsin0 is TSA on NIMA */
 + tsin0: port@0 {
 + tsin-num= 0;
 + serial-not-parallel;
 + i2c-bus = ssc2;
 + rst-gpio= pio15 4 0;

reset-gpios?

Use the GPIO DEFINES.

 + dvb-card= STV0367_TDA18212_NIMA_1;
 + };
 + };
   };
  };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 1/5] ARM: DT: STi: stihxxx-b2120: Add pulse-width properties to ssc2 ssc3

2015-08-28 Thread Lee Jones
On Thu, 27 Aug 2015, Peter Griffin wrote:

 Adding these properties makes the I2C bus to the demodulators much
 more reliable, and we no longer suffer from I2C errors when tuning.
 
 Signed-off-by: Peter Griffin peter.grif...@linaro.org
 ---
  arch/arm/boot/dts/stihxxx-b2120.dtsi | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

Acked-by: Lee Jones lee.jo...@linaro.org

 diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi 
 b/arch/arm/boot/dts/stihxxx-b2120.dtsi
 index f589fe4..62994ae 100644
 --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
 +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
 @@ -27,12 +27,18 @@
   };
   };
  
 - i2c@9842000 {
 + ssc2: i2c@9842000 {
   status = okay;
 + clock-frequency = 10;
 + st,i2c-min-scl-pulse-width-us = 0;
 + st,i2c-min-sda-pulse-width-us = 5;
   };
  
 - i2c@9843000 {
 + ssc3: i2c@9843000 {
   status = okay;
 + clock-frequency = 10;
 + st,i2c-min-scl-pulse-width-us = 0;
 + st,i2c-min-sda-pulse-width-us = 5;
   };
  
   i2c@9844000 {

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 02/10] DT: Add documentation for the mfd Maxim max77693

2015-04-29 Thread Lee Jones
On Tue, 28 Apr 2015, Jacek Anaszewski wrote:

 This patch adds device tree binding documentation for
 the flash cell of the Maxim max77693 multifunctional device.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Acked-by: Sakari Ailus sakari.ai...@linux.intel.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: devicet...@vger.kernel.org
 ---
  Documentation/devicetree/bindings/mfd/max77693.txt |   67 
 
  1 file changed, 67 insertions(+)

Applied with Bryan's Ack.

 diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt 
 b/Documentation/devicetree/bindings/mfd/max77693.txt
 index 38e6440..d342584 100644
 --- a/Documentation/devicetree/bindings/mfd/max77693.txt
 +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
 @@ -76,7 +76,60 @@ Optional properties:
  Valid values: 430, 470, 480, 490
  Default: 430
  
 +- led : the LED submodule device node
 +
 +There are two LED outputs available - FLED1 and FLED2. Each of them can
 +control a separate LED or they can be connected together to double
 +the maximum current for a single connected LED. One LED is represented
 +by one child node.
 +
 +Required properties:
 +- compatible : Must be maxim,max77693-led.
 +
 +Optional properties:
 +- maxim,boost-mode :
 + In boost mode the device can produce up to 1.2A of total current
 + on both outputs. The maximum current on each output is reduced
 + to 625mA then. If not enabled explicitly, boost setting defaults to
 + LEDS_BOOST_FIXED in case both current sources are used.
 + Possible values:
 + LEDS_BOOST_OFF (0) - no boost,
 + LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
 + LEDS_BOOST_FIXED (2) - fixed mode.
 +- maxim,boost-mvout : Output voltage of the boost module in millivolts.
 + Valid values: 3300 - 5500, step by 25 (rounded down)
 + Default: 3300
 +- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
 + if chip estimates that system voltage could drop below this level due
 + to flash power consumption.
 + Valid values: 2400 - 3400, step by 33 (rounded down)
 + Default: 2400
 +
 +Required properties for the LED child node:
 +- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
 + device current output identifiers: 0 - FLED1, 1 - FLED2
 +- led-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Valid values for a LED connected to one FLED output:
 + 15625 - 25, step by 15625 (rounded down)
 + Valid values for a LED connected to both FLED outputs:
 + 15625 - 50, step by 15625 (rounded down)
 +- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Valid values for a single LED connected to one FLED output
 + (boost mode must be turned off):
 + 15625 - 100, step by 15625 (rounded down)
 + Valid values for a single LED connected to both FLED outputs:
 + 15625 - 125, step by 15625 (rounded down)
 + Valid values for two LEDs case:
 + 15625 - 625000, step by 15625 (rounded down)
 +- flash-max-timeout-us : see 
 Documentation/devicetree/bindings/leds/common.txt
 + Valid values: 62500 - 100, step by 62500 (rounded down)
 +
 +Optional properties for the LED child node:
 +- label : see Documentation/devicetree/bindings/leds/common.txt
 +
  Example:
 +#include dt-bindings/leds/common.h
 +
   max77693@66 {
   compatible = maxim,max77693;
   reg = 0x66;
 @@ -117,5 +170,19 @@ Example:
   maxim,thermal-regulation-celsius = 75;
   maxim,battery-overcurrent-microamp = 300;
   maxim,charge-input-threshold-microvolt = 430;
 +
 + led {
 + compatible = maxim,max77693-led;
 + maxim,boost-mode = LEDS_BOOST_FIXED;
 + maxim,boost-mvout = 5000;
 + maxim,mvsys-min = 2400;
 +
 + camera_flash: flash-led {
 + label = max77693-flash;
 + led-sources = 0, 1;
 + led-max-microamp = 50;
 + flash-max-microamp = 125;
 + flash-max-timeout-us = 100;
 + };
   };
   };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 02/10] DT: Add documentation for the mfd Maxim max77693

2015-04-29 Thread Lee Jones
On Wed, 29 Apr 2015, Jacek Anaszewski wrote:

 On 04/29/2015 02:34 PM, Lee Jones wrote:
 LED Ack please Bryan.
 
 You've already applied v6 with Bryan's ack today :)

Ah, this is the same patch?  Jolly good!

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 02/10] DT: Add documentation for the mfd Maxim max77693

2015-04-29 Thread Lee Jones
LED Ack please Bryan.

 This patch adds device tree binding documentation for
 the flash cell of the Maxim max77693 multifunctional device.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: devicet...@vger.kernel.org
 ---
  Documentation/devicetree/bindings/mfd/max77693.txt |   67 
 
  1 file changed, 67 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt 
 b/Documentation/devicetree/bindings/mfd/max77693.txt
 index 38e6440..d342584 100644
 --- a/Documentation/devicetree/bindings/mfd/max77693.txt
 +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
 @@ -76,7 +76,60 @@ Optional properties:
  Valid values: 430, 470, 480, 490
  Default: 430
  
 +- led : the LED submodule device node
 +
 +There are two LED outputs available - FLED1 and FLED2. Each of them can
 +control a separate LED or they can be connected together to double
 +the maximum current for a single connected LED. One LED is represented
 +by one child node.
 +
 +Required properties:
 +- compatible : Must be maxim,max77693-led.
 +
 +Optional properties:
 +- maxim,boost-mode :
 + In boost mode the device can produce up to 1.2A of total current
 + on both outputs. The maximum current on each output is reduced
 + to 625mA then. If not enabled explicitly, boost setting defaults to
 + LEDS_BOOST_FIXED in case both current sources are used.
 + Possible values:
 + LEDS_BOOST_OFF (0) - no boost,
 + LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
 + LEDS_BOOST_FIXED (2) - fixed mode.
 +- maxim,boost-mvout : Output voltage of the boost module in millivolts.
 + Valid values: 3300 - 5500, step by 25 (rounded down)
 + Default: 3300
 +- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
 + if chip estimates that system voltage could drop below this level due
 + to flash power consumption.
 + Valid values: 2400 - 3400, step by 33 (rounded down)
 + Default: 2400
 +
 +Required properties for the LED child node:
 +- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
 + device current output identifiers: 0 - FLED1, 1 - FLED2
 +- led-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Valid values for a LED connected to one FLED output:
 + 15625 - 25, step by 15625 (rounded down)
 + Valid values for a LED connected to both FLED outputs:
 + 15625 - 50, step by 15625 (rounded down)
 +- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Valid values for a single LED connected to one FLED output:
 + (boost mode must be turned off):
 + 15625 - 100, step by 15625 (rounded down)
 + Valid values for a single LED connected to both FLED outputs:
 + 15625 - 125, step by 15625 (rounded down)
 + Valid values for two LEDs case:
 + 15625 - 625000, step by 15625 (rounded down)
 +- flash-max-timeout-us : see 
 Documentation/devicetree/bindings/leds/common.txt
 + Valid values: 62500 - 100, step by 62500 (rounded down)
 +
 +Optional properties for the LED child node:
 +- label : see Documentation/devicetree/bindings/leds/common.txt
 +
  Example:
 +#include dt-bindings/leds/common.h
 +
   max77693@66 {
   compatible = maxim,max77693;
   reg = 0x66;
 @@ -117,5 +170,19 @@ Example:
   maxim,thermal-regulation-celsius = 75;
   maxim,battery-overcurrent-microamp = 300;
   maxim,charge-input-threshold-microvolt = 430;
 +
 + led {
 + compatible = maxim,max77693-led;
 + maxim,boost-mode = LEDS_BOOST_FIXED;
 + maxim,boost-mvout = 5000;
 + maxim,mvsys-min = 2400;
 +
 + camera_flash: flash-led {
 + label = max77693-flash;
 + led-sources = 0, 1;
 + led-max-microamp = 50;
 + flash-max-microamp = 125;
 + flash-max-timeout-us = 100;
 + };
   };
   };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 02/10] DT: Add documentation for the mfd Maxim max77693

2015-04-28 Thread Lee Jones
On Tue, 28 Apr 2015, Jacek Anaszewski wrote:

 This patch adds device tree binding documentation for
 the flash cell of the Maxim max77693 multifunctional device.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Acked-by: Sakari Ailus sakari.ai...@linux.intel.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: devicet...@vger.kernel.org
 ---
  Documentation/devicetree/bindings/mfd/max77693.txt |   67 
 
  1 file changed, 67 insertions(+)

Requires a LED Ack.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v13 04/13] DT: Add documentation for the mfd Maxim max77693

2015-04-02 Thread Lee Jones
On Thu, 02 Apr 2015, Pavel Machek wrote:

 On Mon 2015-03-23 15:59:37, Lee Jones wrote:
  On Mon, 23 Mar 2015, Pavel Machek wrote:
  
   On Mon 2015-03-23 15:02:13, Lee Jones wrote:
On Mon, 23 Mar 2015, Pavel Machek wrote:
 On Mon 2015-03-23 12:07:43, Lee Jones wrote:
  This patch requires a DT Ack.
 
 No, it requires DT people to be notified -- and they were, few times
 by now.
 
 They clearly don't care.

Well fortunately for the Kernel community, I do care.  And as this
patch adds 3 new DT properties, has been through many iterations
already with vast changes made over that period and there is still
some controversy looming, I'm saying that it _does_ require a DT
Ack.
   
   Can you help get that ack, then? As a maintainer, you have better
   chance getting reply from DT people than patch submitter.
  
  Hopefully they will see my plea a couple of replies back.
  
  If that fails I'll go poke them via other means.  Failing that I'll go
  see them in person and continually hit them with wet fish until one of
  them relents.
 
 Can I request footage of the last steps on the youtube?

Too late, it's been and gone.

 (IOW ping, I don't think I seen any replies from DT maintainers...)

Then you haven't been looking the right place [1]. ;)

[1] http://comments.gmane.org/gmane.linux.leds/2245

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1 02/11] DT: Add documentation for the mfd Maxim max77693

2015-03-26 Thread Lee Jones
On Fri, 20 Mar 2015, Jacek Anaszewski wrote:

 This patch adds device tree binding documentation for
 the flash cell of the Maxim max77693 multifunctional device.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 ---
  Documentation/devicetree/bindings/mfd/max77693.txt |   61 
 
  1 file changed, 61 insertions(+)

Bryan and/or one of the DT folks really need to Ack this.

 diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt 
 b/Documentation/devicetree/bindings/mfd/max77693.txt
 index 38e6440..15c546e 100644
 --- a/Documentation/devicetree/bindings/mfd/max77693.txt
 +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
 @@ -76,7 +76,53 @@ Optional properties:
  Valid values: 430, 470, 480, 490
  Default: 430
  
 +- led : the LED submodule device node
 +
 +There are two LED outputs available - FLED1 and FLED2. Each of them can
 +control a separate LED or they can be connected together to double
 +the maximum current for a single connected LED. One LED is represented
 +by one child node.
 +
 +Required properties:
 +- compatible : Must be maxim,max77693-led.
 +
 +Optional properties:
 +- maxim,trigger-type : Flash trigger type.
 + Possible trigger types:
 + LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
 + the flash,
 + LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
 + of the flash.
 +- maxim,boost-mode :
 + In boost mode the device can produce up to 1.2A of total current
 + on both outputs. The maximum current on each output is reduced
 + to 625mA then. If not enabled explicitly, boost setting defaults to
 + LEDS_BOOST_FIXED in case both current sources are used.
 + Possible values:
 + LEDS_BOOST_OFF (0) - no boost,
 + LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
 + LEDS_BOOST_FIXED (2) - fixed mode.
 +- maxim,boost-mvout : Output voltage of the boost module in millivolts.
 +- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
 + if chip estimates that system voltage could drop below this level due
 + to flash power consumption.
 +
 +Required properties of the LED child node:
 +- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
 + device current output identifiers: 0 - FLED1, 1 - FLED2
 +
 +Optional properties of the LED child node:
 +- label : see Documentation/devicetree/bindings/leds/common.txt
 +- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 15625 - 25
 +- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 15625 - 100
 +- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 62500 - 100
 +
  Example:
 +#include dt-bindings/leds/common.h
 +
   max77693@66 {
   compatible = maxim,max77693;
   reg = 0x66;
 @@ -117,5 +163,20 @@ Example:
   maxim,thermal-regulation-celsius = 75;
   maxim,battery-overcurrent-microamp = 300;
   maxim,charge-input-threshold-microvolt = 430;
 +
 + led {
 + compatible = maxim,max77693-led;
 + maxim,trigger-type = LEDS_TRIG_TYPE_LEVEL;
 + maxim,boost-mode = LEDS_BOOST_FIXED;
 + maxim,boost-mvout = 5000;
 + maxim,mvsys-min = 2400;
 +
 + camera_flash: flash-led {
 + label = max77693-flash;
 + led-sources = 0, 1;
 + max-microamp = 50;
 + flash-max-microamp = 125;
 + flash-timeout-us = 100;
 + };
   };
   };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v13 03/13] leds: Add support for max77693 mfd flash cell

2015-03-23 Thread Lee Jones
 This patch requires a DT Ack.

Whoops, not this one.  Patch 4.

  This patch adds led-flash support to Maxim max77693 chipset.
  A device can be exposed to user space through LED subsystem
  sysfs interface. Device supports up to two leds which can
  work in flash and torch mode. The leds can be triggered
  externally or by software.
  
  Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
  Signed-off-by: Andrzej Hajda a.ha...@samsung.com
  Acked-by: Kyungmin Park kyungmin.p...@samsung.com
  Cc: Bryan Wu coolo...@gmail.com
  Cc: Richard Purdie rpur...@rpsys.net
  Cc: Lee Jones lee.jo...@linaro.org
  Cc: Chanwoo Choi cw00.c...@samsung.com
  ---
   drivers/leds/Kconfig |   10 +
   drivers/leds/Makefile|1 +
   drivers/leds/leds-max77693.c |  993 
  ++
   3 files changed, 1004 insertions(+)
   create mode 100644 drivers/leds/leds-max77693.c
  
  diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
  index 25b320d..ccef436 100644
  --- a/drivers/leds/Kconfig
  +++ b/drivers/leds/Kconfig
  @@ -467,6 +467,16 @@ config LEDS_TCA6507
LED driver chips accessed via the I2C bus.
Driver support brightness control and hardware-assisted blinking.
   
  +config LEDS_MAX77693
  +   tristate LED support for MAX77693 Flash
  +   depends on LEDS_CLASS_FLASH
  +   depends on MFD_MAX77693
  +   depends on OF
  +   help
  + This option enables support for the flash part of the MAX77693
  + multifunction device. It has build in control for two leds in flash
  + and torch mode.
  +
   config LEDS_MAX8997
  tristate LED support for MAX8997 PMIC
  depends on LEDS_CLASS  MFD_MAX8997
  diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
  index cbba921..57ca62b 100644
  --- a/drivers/leds/Makefile
  +++ b/drivers/leds/Makefile
  @@ -52,6 +52,7 @@ obj-$(CONFIG_LEDS_MC13783)+= 
  leds-mc13783.o
   obj-$(CONFIG_LEDS_NS2) += leds-ns2.o
   obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
   obj-$(CONFIG_LEDS_ASIC3)   += leds-asic3.o
  +obj-$(CONFIG_LEDS_MAX77693)+= leds-max77693.o
   obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
   obj-$(CONFIG_LEDS_LM355x)  += leds-lm355x.o
   obj-$(CONFIG_LEDS_BLINKM)  += leds-blinkm.o
  diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
  new file mode 100644
  index 000..6873611
  --- /dev/null
  +++ b/drivers/leds/leds-max77693.c
  @@ -0,0 +1,993 @@
  +/*
  + * LED Flash class driver for the flash cell of max77693 mfd.
  + *
  + * Copyright (C) 2015, Samsung Electronics Co., Ltd.
  + *
  + * Authors: Jacek Anaszewski j.anaszew...@samsung.com
  + *  Andrzej Hajda a.ha...@samsung.com
  + *
  + * This program is free software; you can redistribute it and/or
  + * modify it under the terms of the GNU General Public License
  + * version 2 as published by the Free Software Foundation.
  + */
  +
  +#include asm/div64.h
  +#include linux/led-class-flash.h
  +#include linux/mfd/max77693.h
  +#include linux/mfd/max77693-private.h
  +#include linux/module.h
  +#include linux/mutex.h
  +#include linux/platform_device.h
  +#include linux/regmap.h
  +#include linux/slab.h
  +#include linux/workqueue.h
  +
  +#define MODE_OFF   0
  +#define MODE_FLASH(a)  (1  (a))
  +#define MODE_TORCH(a)  (1  (2 + (a)))
  +#define MODE_FLASH_EXTERNAL(a) (1  (4 + (a)))
  +
  +#define MODE_FLASH_MASK(MODE_FLASH(FLED1) | MODE_FLASH(FLED2) 
  | \
  +MODE_FLASH_EXTERNAL(FLED1) | \
  +MODE_FLASH_EXTERNAL(FLED2))
  +#define MODE_TORCH_MASK(MODE_TORCH(FLED1) | MODE_TORCH(FLED2))
  +
  +#define FLED1_IOUT (1  0)
  +#define FLED2_IOUT (1  1)
  +
  +#define MAX77693_LED1_NAME max77693-led.1
  +#define MAX77693_LED2_NAME max77693-led.2
  +
  +enum max77693_fled {
  +   FLED1,
  +   FLED2,
  +};
  +
  +enum max77693_led_mode {
  +   FLASH,
  +   TORCH,
  +};
  +
  +struct max77693_led_config_data {
  +   const char *label[2];
  +   u32 iout_torch_max[2];
  +   u32 iout_flash_max[2];
  +   u32 flash_timeout[2];
  +   u32 num_leds;
  +   u32 boost_mode;
  +   u32 boost_vout;
  +   u32 low_vsys;
  +   u32 trigger_type;
  +};
  +
  +struct max77693_sub_led {
  +   /* related FLED output identifier */
  +   int fled_id;
  +   /* related LED Flash class device */
  +   struct led_classdev_flash fled_cdev;
  +   /* assures led-triggers compatibility */
  +   struct work_struct work_brightness_set;
  +
  +   /* brightness cache */
  +   unsigned int torch_brightness;
  +   /* flash timeout cache */
  +   unsigned int flash_timeout;
  +   /* flash faults that may have occurred */
  +   u32 flash_faults;
  +};
  +
  +struct max77693_led_device {
  +   /* parent mfd regmap */
  +   struct regmap *regmap;
  +   /* platform device data */
  +   struct platform_device *pdev;
  +   /* secures

Re: [PATCH/RFC v13 04/13] DT: Add documentation for the mfd Maxim max77693

2015-03-23 Thread Lee Jones
This patch requires a DT Ack.

 This patch adds device tree binding documentation for
 the flash cell of the Maxim max77693 multifunctional device.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 ---
  Documentation/devicetree/bindings/mfd/max77693.txt |   61 
 
  1 file changed, 61 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt 
 b/Documentation/devicetree/bindings/mfd/max77693.txt
 index 38e6440..15c546e 100644
 --- a/Documentation/devicetree/bindings/mfd/max77693.txt
 +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
 @@ -76,7 +76,53 @@ Optional properties:
  Valid values: 430, 470, 480, 490
  Default: 430
  
 +- led : the LED submodule device node
 +
 +There are two LED outputs available - FLED1 and FLED2. Each of them can
 +control a separate LED or they can be connected together to double
 +the maximum current for a single connected LED. One LED is represented
 +by one child node.
 +
 +Required properties:
 +- compatible : Must be maxim,max77693-led.
 +
 +Optional properties:
 +- maxim,trigger-type : Flash trigger type.
 + Possible trigger types:
 + LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
 + the flash,
 + LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
 + of the flash.
 +- maxim,boost-mode :
 + In boost mode the device can produce up to 1.2A of total current
 + on both outputs. The maximum current on each output is reduced
 + to 625mA then. If not enabled explicitly, boost setting defaults to
 + LEDS_BOOST_FIXED in case both current sources are used.
 + Possible values:
 + LEDS_BOOST_OFF (0) - no boost,
 + LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
 + LEDS_BOOST_FIXED (2) - fixed mode.
 +- maxim,boost-mvout : Output voltage of the boost module in millivolts.
 +- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not fired
 + if chip estimates that system voltage could drop below this level due
 + to flash power consumption.
 +
 +Required properties of the LED child node:
 +- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
 + device current output identifiers: 0 - FLED1, 1 - FLED2
 +
 +Optional properties of the LED child node:
 +- label : see Documentation/devicetree/bindings/leds/common.txt
 +- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 15625 - 25
 +- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 15625 - 100
 +- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 62500 - 100
 +
  Example:
 +#include dt-bindings/leds/common.h
 +
   max77693@66 {
   compatible = maxim,max77693;
   reg = 0x66;
 @@ -117,5 +163,20 @@ Example:
   maxim,thermal-regulation-celsius = 75;
   maxim,battery-overcurrent-microamp = 300;
   maxim,charge-input-threshold-microvolt = 430;
 +
 + led {
 + compatible = maxim,max77693-led;
 + maxim,trigger-type = LEDS_TRIG_TYPE_LEVEL;
 + maxim,boost-mode = LEDS_BOOST_FIXED;
 + maxim,boost-mvout = 5000;
 + maxim,mvsys-min = 2400;
 +
 + camera_flash: flash-led {
 + label = max77693-flash;
 + led-sources = 0, 1;
 + max-microamp = 50;
 + flash-max-microamp = 125;
 + flash-timeout-us = 100;
 + };
   };
   };


-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v13 04/13] DT: Add documentation for the mfd Maxim max77693

2015-03-23 Thread Lee Jones
On Mon, 23 Mar 2015, Pavel Machek wrote:
 On Mon 2015-03-23 12:07:43, Lee Jones wrote:
  This patch requires a DT Ack.
 
 No, it requires DT people to be notified -- and they were, few times
 by now.
 
 They clearly don't care.

Well fortunately for the Kernel community, I do care.  And as this
patch adds 3 new DT properties, has been through many iterations
already with vast changes made over that period and there is still
some controversy looming, I'm saying that it _does_ require a DT Ack.

   This patch adds device tree binding documentation for
   the flash cell of the Maxim max77693 multifunctional device.
   
   Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
   Signed-off-by: Andrzej Hajda a.ha...@samsung.com
   Acked-by: Kyungmin Park kyungmin.p...@samsung.com
   Cc: Lee Jones lee.jo...@linaro.org
   Cc: Chanwoo Choi cw00.c...@samsung.com
   Cc: Bryan Wu coolo...@gmail.com
   Cc: Richard Purdie rpur...@rpsys.net
 
 Acked-by: Pavel Machek pa...@ucw.cz
 
 
   diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt 
   b/Documentation/devicetree/bindings/mfd/max77693.txt
   index 38e6440..15c546e 100644
   --- a/Documentation/devicetree/bindings/mfd/max77693.txt
   +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
   @@ -76,7 +76,53 @@ Optional properties:
Valid values: 430, 470, 480, 490
Default: 430

   +- led : the LED submodule device node
   +
   +There are two LED outputs available - FLED1 and FLED2. Each of them can
   +control a separate LED or they can be connected together to double
   +the maximum current for a single connected LED. One LED is represented
   +by one child node.
   +
   +Required properties:
   +- compatible : Must be maxim,max77693-led.
   +
   +Optional properties:
   +- maxim,trigger-type : Flash trigger type.
   + Possible trigger types:
   + LEDS_TRIG_TYPE_EDGE (0) - Rising edge of the signal triggers
   + the flash,
   + LEDS_TRIG_TYPE_LEVEL (1) - Strobe pulse length controls duration
   + of the flash.
   +- maxim,boost-mode :
   + In boost mode the device can produce up to 1.2A of total current
   + on both outputs. The maximum current on each output is reduced
   + to 625mA then. If not enabled explicitly, boost setting defaults to
   + LEDS_BOOST_FIXED in case both current sources are used.
   + Possible values:
   + LEDS_BOOST_OFF (0) - no boost,
   + LEDS_BOOST_ADAPTIVE (1) - adaptive mode,
   + LEDS_BOOST_FIXED (2) - fixed mode.
   +- maxim,boost-mvout : Output voltage of the boost module in millivolts.
   +- maxim,mvsys-min : Low input voltage level in millivolts. Flash is not 
   fired
   + if chip estimates that system voltage could drop below this level due
   + to flash power consumption.
   +
   +Required properties of the LED child node:
   +- led-sources : see Documentation/devicetree/bindings/leds/common.txt;
   + device current output identifiers: 0 - FLED1, 1 - FLED2
   +
   +Optional properties of the LED child node:
   +- label : see Documentation/devicetree/bindings/leds/common.txt
   +- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
   + Range: 15625 - 25
   +- flash-max-microamp : see 
   Documentation/devicetree/bindings/leds/common.txt
   + Range: 15625 - 100
   +- flash-timeout-us : see 
   Documentation/devicetree/bindings/leds/common.txt
   + Range: 62500 - 100
   +
Example:
   +#include dt-bindings/leds/common.h
   +
 max77693@66 {
 compatible = maxim,max77693;
 reg = 0x66;
   @@ -117,5 +163,20 @@ Example:
 maxim,thermal-regulation-celsius = 75;
 maxim,battery-overcurrent-microamp = 300;
 maxim,charge-input-threshold-microvolt = 430;
   +
   + led {
   + compatible = maxim,max77693-led;
   + maxim,trigger-type = LEDS_TRIG_TYPE_LEVEL;
   + maxim,boost-mode = LEDS_BOOST_FIXED;
   + maxim,boost-mvout = 5000;
   + maxim,mvsys-min = 2400;
   +
   + camera_flash: flash-led {
   + label = max77693-flash;
   + led-sources = 0, 1;
   + max-microamp = 50;
   + flash-max-microamp = 125;
   + flash-timeout-us = 100;
   + };
 };
 };
  
  
 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v13 04/13] DT: Add documentation for the mfd Maxim max77693

2015-03-23 Thread Lee Jones
On Mon, 23 Mar 2015, Pavel Machek wrote:

 On Mon 2015-03-23 15:02:13, Lee Jones wrote:
  On Mon, 23 Mar 2015, Pavel Machek wrote:
   On Mon 2015-03-23 12:07:43, Lee Jones wrote:
This patch requires a DT Ack.
   
   No, it requires DT people to be notified -- and they were, few times
   by now.
   
   They clearly don't care.
  
  Well fortunately for the Kernel community, I do care.  And as this
  patch adds 3 new DT properties, has been through many iterations
  already with vast changes made over that period and there is still
  some controversy looming, I'm saying that it _does_ require a DT
  Ack.
 
 Can you help get that ack, then? As a maintainer, you have better
 chance getting reply from DT people than patch submitter.

Hopefully they will see my plea a couple of replies back.

If that fails I'll go poke them via other means.  Failing that I'll go
see them in person and continually hit them with wet fish until one of
them relents.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v13 03/13] leds: Add support for max77693 mfd flash cell

2015-03-23 Thread Lee Jones
This patch requires a DT Ack.

 This patch adds led-flash support to Maxim max77693 chipset.
 A device can be exposed to user space through LED subsystem
 sysfs interface. Device supports up to two leds which can
 work in flash and torch mode. The leds can be triggered
 externally or by software.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 ---
  drivers/leds/Kconfig |   10 +
  drivers/leds/Makefile|1 +
  drivers/leds/leds-max77693.c |  993 
 ++
  3 files changed, 1004 insertions(+)
  create mode 100644 drivers/leds/leds-max77693.c
 
 diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
 index 25b320d..ccef436 100644
 --- a/drivers/leds/Kconfig
 +++ b/drivers/leds/Kconfig
 @@ -467,6 +467,16 @@ config LEDS_TCA6507
 LED driver chips accessed via the I2C bus.
 Driver support brightness control and hardware-assisted blinking.
  
 +config LEDS_MAX77693
 + tristate LED support for MAX77693 Flash
 + depends on LEDS_CLASS_FLASH
 + depends on MFD_MAX77693
 + depends on OF
 + help
 +   This option enables support for the flash part of the MAX77693
 +   multifunction device. It has build in control for two leds in flash
 +   and torch mode.
 +
  config LEDS_MAX8997
   tristate LED support for MAX8997 PMIC
   depends on LEDS_CLASS  MFD_MAX8997
 diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
 index cbba921..57ca62b 100644
 --- a/drivers/leds/Makefile
 +++ b/drivers/leds/Makefile
 @@ -52,6 +52,7 @@ obj-$(CONFIG_LEDS_MC13783)  += leds-mc13783.o
  obj-$(CONFIG_LEDS_NS2)   += leds-ns2.o
  obj-$(CONFIG_LEDS_NETXBIG)   += leds-netxbig.o
  obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o
 +obj-$(CONFIG_LEDS_MAX77693)  += leds-max77693.o
  obj-$(CONFIG_LEDS_MAX8997)   += leds-max8997.o
  obj-$(CONFIG_LEDS_LM355x)+= leds-lm355x.o
  obj-$(CONFIG_LEDS_BLINKM)+= leds-blinkm.o
 diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
 new file mode 100644
 index 000..6873611
 --- /dev/null
 +++ b/drivers/leds/leds-max77693.c
 @@ -0,0 +1,993 @@
 +/*
 + * LED Flash class driver for the flash cell of max77693 mfd.
 + *
 + *   Copyright (C) 2015, Samsung Electronics Co., Ltd.
 + *
 + *   Authors: Jacek Anaszewski j.anaszew...@samsung.com
 + *Andrzej Hajda a.ha...@samsung.com
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License
 + * version 2 as published by the Free Software Foundation.
 + */
 +
 +#include asm/div64.h
 +#include linux/led-class-flash.h
 +#include linux/mfd/max77693.h
 +#include linux/mfd/max77693-private.h
 +#include linux/module.h
 +#include linux/mutex.h
 +#include linux/platform_device.h
 +#include linux/regmap.h
 +#include linux/slab.h
 +#include linux/workqueue.h
 +
 +#define MODE_OFF 0
 +#define MODE_FLASH(a)(1  (a))
 +#define MODE_TORCH(a)(1  (2 + (a)))
 +#define MODE_FLASH_EXTERNAL(a)   (1  (4 + (a)))
 +
 +#define MODE_FLASH_MASK  (MODE_FLASH(FLED1) | MODE_FLASH(FLED2) 
 | \
 +  MODE_FLASH_EXTERNAL(FLED1) | \
 +  MODE_FLASH_EXTERNAL(FLED2))
 +#define MODE_TORCH_MASK  (MODE_TORCH(FLED1) | MODE_TORCH(FLED2))
 +
 +#define FLED1_IOUT   (1  0)
 +#define FLED2_IOUT   (1  1)
 +
 +#define MAX77693_LED1_NAME   max77693-led.1
 +#define MAX77693_LED2_NAME   max77693-led.2
 +
 +enum max77693_fled {
 + FLED1,
 + FLED2,
 +};
 +
 +enum max77693_led_mode {
 + FLASH,
 + TORCH,
 +};
 +
 +struct max77693_led_config_data {
 + const char *label[2];
 + u32 iout_torch_max[2];
 + u32 iout_flash_max[2];
 + u32 flash_timeout[2];
 + u32 num_leds;
 + u32 boost_mode;
 + u32 boost_vout;
 + u32 low_vsys;
 + u32 trigger_type;
 +};
 +
 +struct max77693_sub_led {
 + /* related FLED output identifier */
 + int fled_id;
 + /* related LED Flash class device */
 + struct led_classdev_flash fled_cdev;
 + /* assures led-triggers compatibility */
 + struct work_struct work_brightness_set;
 +
 + /* brightness cache */
 + unsigned int torch_brightness;
 + /* flash timeout cache */
 + unsigned int flash_timeout;
 + /* flash faults that may have occurred */
 + u32 flash_faults;
 +};
 +
 +struct max77693_led_device {
 + /* parent mfd regmap */
 + struct regmap *regmap;
 + /* platform device data */
 + struct platform_device *pdev;
 + /* secures access to the device */
 + struct mutex lock;
 +
 + /* sub led

Re: [PATCH/RFC v12 08/19] mfd: max77693: Adjust FLASH_EN_SHIFT and TORCH_EN_SHIFT macros

2015-03-09 Thread Lee Jones
On Wed, 04 Mar 2015, Jacek Anaszewski wrote:

 Modify FLASH_EN_SHIFT and TORCH_EN_SHIFT macros to work properly
 when passed enum max77693_fled values (0 for FLED1 and 1 for FLED2)
 from leds-max77693 driver. Previous definitions were compatible with
 one of the previous RFC versions of leds-max77693.c driver, which was
 not merged.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  include/linux/mfd/max77693-private.h |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

 diff --git a/include/linux/mfd/max77693-private.h 
 b/include/linux/mfd/max77693-private.h
 index 8770ce1..51633ea 100644
 --- a/include/linux/mfd/max77693-private.h
 +++ b/include/linux/mfd/max77693-private.h
 @@ -114,8 +114,8 @@ enum max77693_pmic_reg {
  #define FLASH_EN_FLASH   0x1
  #define FLASH_EN_TORCH   0x2
  #define FLASH_EN_ON  0x3
 -#define FLASH_EN_SHIFT(x)(6 - ((x) - 1) * 2)
 -#define TORCH_EN_SHIFT(x)(2 - ((x) - 1) * 2)
 +#define FLASH_EN_SHIFT(x)(6 - (x) * 2)
 +#define TORCH_EN_SHIFT(x)(2 - (x) * 2)
  
  /* MAX77693 MAX_FLASH1 register */
  #define MAX_FLASH1_MAX_FL_EN 0x80

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v12 06/19] mfd: max77693: Remove struct max77693_led_platform_data

2015-03-09 Thread Lee Jones
On Wed, 04 Mar 2015, Jacek Anaszewski wrote:

 The flash part of the max77693 device will depend only on OF, and thus
 will not use board files. Since there are no other users of the
 struct max77693_led_platform_data its existence is unjustified.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  include/linux/mfd/max77693.h |   13 -
  1 file changed, 13 deletions(-)

Applied, thanks.

 diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
 index f0b6585..ce894b6 100644
 --- a/include/linux/mfd/max77693.h
 +++ b/include/linux/mfd/max77693.h
 @@ -87,19 +87,6 @@ enum max77693_led_boost_mode {
   MAX77693_LED_BOOST_FIXED,
  };
  
 -struct max77693_led_platform_data {
 - u32 fleds[2];
 - u32 iout_torch[2];
 - u32 iout_flash[2];
 - u32 trigger[2];
 - u32 trigger_type[2];
 - u32 num_leds;
 - u32 boost_mode;
 - u32 flash_timeout;
 - u32 boost_vout;
 - u32 low_vsys;
 -};
 -
  /* MAX77693 */
  
  struct max77693_platform_data {

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v12 07/19] mfd: max77693: add TORCH_IOUT_MASK macro

2015-03-09 Thread Lee Jones
On Wed, 04 Mar 2015, Jacek Anaszewski wrote:

 Add a macro for obtaining the mask of ITORCH register bit fields
 related either to FLED1 or FLED2 current output. The expected
 arguments are TORCH_IOUT1_SHIFT or TORCH_IOUT2_SHIFT.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  include/linux/mfd/max77693-private.h |1 +
  1 file changed, 1 insertion(+)

Applied, thanks.

 diff --git a/include/linux/mfd/max77693-private.h 
 b/include/linux/mfd/max77693-private.h
 index 955dd99..8770ce1 100644
 --- a/include/linux/mfd/max77693-private.h
 +++ b/include/linux/mfd/max77693-private.h
 @@ -87,6 +87,7 @@ enum max77693_pmic_reg {
  /* MAX77693 ITORCH register */
  #define TORCH_IOUT1_SHIFT0
  #define TORCH_IOUT2_SHIFT4
 +#define TORCH_IOUT_MASK(x)   (0xf  (x))
  #define TORCH_IOUT_MIN   15625
  #define TORCH_IOUT_MAX   25
  #define TORCH_IOUT_STEP  15625

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v12 05/19] mfd: max77693: Modify flash cell name identifiers

2015-03-09 Thread Lee Jones
On Wed, 04 Mar 2015, Jacek Anaszewski wrote:

 Change flash cell identifiers from max77693-flash to max77693-led
 to avoid confusion with NOR/NAND Flash.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Acked-by: Lee Jones lee.jo...@linaro.org
 ---
  drivers/mfd/max77693.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

 diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
 index a159593..cb14afa 100644
 --- a/drivers/mfd/max77693.c
 +++ b/drivers/mfd/max77693.c
 @@ -53,8 +53,8 @@ static const struct mfd_cell max77693_devs[] = {
   .of_compatible = maxim,max77693-haptic,
   },
   {
 - .name = max77693-flash,
 - .of_compatible = maxim,max77693-flash,
 + .name = max77693-led,
 + .of_compatible = maxim,max77693-led,
   },
  };
  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v10 04/19] dt-binding: mfd: max77693: Add DT binding related macros

2015-01-20 Thread Lee Jones
On Fri, 09 Jan 2015, Jacek Anaszewski wrote:

 Add macros for max77693 led part related binding.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 ---
  include/dt-bindings/mfd/max77693.h |   21 +
  1 file changed, 21 insertions(+)
  create mode 100644 include/dt-bindings/mfd/max77693.h
 
 diff --git a/include/dt-bindings/mfd/max77693.h 
 b/include/dt-bindings/mfd/max77693.h
 new file mode 100644
 index 000..f53e197
 --- /dev/null
 +++ b/include/dt-bindings/mfd/max77693.h
 @@ -0,0 +1,21 @@
 +/*
 + * This header provides macros for MAX77693 device binding
 + *
 + * Copyright (C) 2014, Samsung Electronics Co., Ltd.
 + *
 + * Author: Jacek Anaszewski j.anaszew...@samsung.com
 + */
 +
 +#ifndef __DT_BINDINGS_MAX77693_H__
 +#define __DT_BINDINGS_MAX77693_H
 +
 +/* External trigger type */
 +#define MAX77693_LED_TRIG_TYPE_EDGE  0
 +#define MAX77693_LED_TRIG_TYPE_LEVEL 1
 +
 +/* Boost modes */
 +#define MAX77693_LED_BOOST_OFF   0
 +#define MAX77693_LED_BOOST_ADAPTIVE  1
 +#define MAX77693_LED_BOOST_FIXED 2
 +
 +#endif /* __DT_BINDINGS_MAX77693_H */

These look fairly generic.  Do generic LED defines already exist?  If
not, can they?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v10 05/19] mfd: max77693: Modify flash cell name identifiers

2015-01-20 Thread Lee Jones
On Fri, 09 Jan 2015, Jacek Anaszewski wrote:

 Change flash cell identifiers from max77693-flash to max77693-led
 to avoid confusion with NOR/NAND Flash.

This is okay by me, but aren't these ABI yet?

 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  drivers/mfd/max77693.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
 index a159593..cb14afa 100644
 --- a/drivers/mfd/max77693.c
 +++ b/drivers/mfd/max77693.c
 @@ -53,8 +53,8 @@ static const struct mfd_cell max77693_devs[] = {
   .of_compatible = maxim,max77693-haptic,
   },
   {
 - .name = max77693-flash,
 - .of_compatible = maxim,max77693-flash,
 + .name = max77693-led,
 + .of_compatible = maxim,max77693-led,
   },
  };
  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v10 07/19] mfd: max77693: Adjust FLASH_EN_SHIFT and TORCH_EN_SHIFT macros

2015-01-20 Thread Lee Jones
On Fri, 09 Jan 2015, Jacek Anaszewski wrote:

 Modify FLASH_EN_SHIFT and TORCH_EN_SHIFT macros to work properly
 when passed enum max77693_fled values (0 for FLED1 and 1 for FLED2)
 from leds-max77693 driver.

Off-by-one ay?  Wasn't the original code tested?

 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  include/linux/mfd/max77693-private.h |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/include/linux/mfd/max77693-private.h 
 b/include/linux/mfd/max77693-private.h
 index 08dae01..01799a9 100644
 --- a/include/linux/mfd/max77693-private.h
 +++ b/include/linux/mfd/max77693-private.h
 @@ -113,8 +113,8 @@ enum max77693_pmic_reg {
  #define FLASH_EN_FLASH   0x1
  #define FLASH_EN_TORCH   0x2
  #define FLASH_EN_ON  0x3
 -#define FLASH_EN_SHIFT(x)(6 - ((x) - 1) * 2)
 -#define TORCH_EN_SHIFT(x)(2 - ((x) - 1) * 2)
 +#define FLASH_EN_SHIFT(x)(6 - (x) * 2)
 +#define TORCH_EN_SHIFT(x)(2 - (x) * 2)
  
  /* MAX77693 MAX_FLASH1 register */
  #define MAX_FLASH1_MAX_FL_EN 0x80

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v10 09/19] DT: Add documentation for the mfd Maxim max77693

2015-01-20 Thread Lee Jones
On Fri, 09 Jan 2015, Jacek Anaszewski wrote:

 This patch adds device tree binding documentation for
 the flash cell of the Maxim max77693 multifunctional device.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: Rob Herring robh...@kernel.org
 Cc: Pawel Moll pawel.m...@arm.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Ian Campbell ijc+devicet...@hellion.org.uk
 Cc: Kumar Gala ga...@codeaurora.org
 ---
  Documentation/devicetree/bindings/mfd/max77693.txt |   69 
 
  1 file changed, 69 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt 
 b/Documentation/devicetree/bindings/mfd/max77693.txt
 index 01e9f30..ef184f0 100644
 --- a/Documentation/devicetree/bindings/mfd/max77693.txt
 +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
 @@ -41,7 +41,52 @@ Optional properties:
To get more informations, please refer to documentaion.
   [*] refer Documentation/devicetree/bindings/pwm/pwm.txt
  
 +- led : the LED submodule device node
 +
 +There are two led outputs available - fled1 and fled2. Each of them can
 +control a separate led or they can be connected together to double
 +the maximum current for a single connected led. One led is represented
 +by one child node.
 +
 +Required properties:
 +- compatible : Must be maxim,max77693-led.
 +
 +Optional properties:
 +- maxim,trigger-type : Flash trigger type.
 + Possible trigger types:
 + MAX77693_LED_TRIG_TYPE_EDGE - Rising edge of the signal triggers
 + the flash,
 + MAX77693_LED_TRIG_TYPE_LEVEL - Strobe pulse length controls
 + duration of the flash.

I think you should represent the proper values here instead of the
defines.

 +- maxim,boost-mode :
 + In boost mode the device can produce up to 1.2A of total current
 + on both outputs. The maximum current on each output is reduced
 + to 625mA then. If not enabled explicitly, boost setting defaults to
 + MAX77693_LED_BOOST_FIXED in case both current sources are used.
 + Possible values:
 + MAX77693_LED_BOOST_OFF - no boost,
 + MAX77693_LED_BOOST_ADAPTIVE - adaptive mode,
 + MAX77693_LED_BOOST_FIXED - fixed mode.

Same here.

 +- maxim,boost-vout : Output voltage of the boost module in millivolts.

-mvout?
-microvout?

 +- maxim,vsys-min : Low input voltage level in millivolts. Flash is not fired
 + if chip estimates that system voltage could drop below this level due
 + to flash power consumption.

mvsys?
microvsys?

 +Required properties of the LED child node:
 +- label : see Documentation/devicetree/bindings/leds/common.txt
 +- led-sources : see Documentation/devicetree/bindings/leds/common.txt
 +
 +Optional properties of the LED child node:
 +- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 15625 - 25
 +- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 15625 - 100
 +- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 62500 - 100
 +
  Example:
 +#include dt-bindings/mfd/max77693.h
 +
   max77693@66 {
   compatible = maxim,max77693;
   reg = 0x66;
 @@ -73,4 +118,28 @@ Example:
   pwms = pwm 0 4 0;
   pwm-names = haptic;
   };
 +
 + led {
 + compatible = maxim,max77693-led;
 + maxim,trigger-type = MAX77693_LED_TRIG_TYPE_LEVEL;
 + maxim,boost-mode = MAX77693_LED_BOOST_FIXED;
 + maxim,boost-vout = 5000;
 + maxim,vsys-min = 2400;
 +
 + camera1_flash: led1 {
 + label = max77693-flash1;
 + led-sources = 1 0;
 + max-microamp = 25;
 + flash-max-microamp = 625000;
 + flash-timeout-us = 100;
 + };
 +
 + camera2_flash: led2 {
 + label = max77693-flash2;
 + led-sources = 0 1;
 + max-microamp = 25;
 + flash-max-microamp = 625000;
 + flash-timeout-us = 100;
 + };
 + };
   };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More

Re: [PATCH/RFC v10 06/19] mfd: max77693: modifications around max77693_led_platform_data

2015-01-20 Thread Lee Jones
On Fri, 09 Jan 2015, Jacek Anaszewski wrote:

 1. Rename max77693_led_platform_data to max77693_led_config_data to
avoid making impression that the led driver expects a board file -
it relies on Device Tree data.
 2. Remove fleds array, as the DT binding design has changed
 3. Add label array for Device Tree strings with the name of a LED device
 4. Make flash_timeout a two element array, for caching the sub-led
related flash timeout.
 5. Remove trigger array as the related data will not be provided
in the DT binding

Code looks fine, and I'm sure you've tested this thoroughly.

I'm slightly concerned about current users though.  Are there any?  Is
this patch-set fully bisectable?

 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  include/linux/mfd/max77693.h |9 -
  1 file changed, 4 insertions(+), 5 deletions(-)
 
 diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
 index f0b6585..c1ccb13 100644
 --- a/include/linux/mfd/max77693.h
 +++ b/include/linux/mfd/max77693.h
 @@ -87,17 +87,16 @@ enum max77693_led_boost_mode {
   MAX77693_LED_BOOST_FIXED,
  };
  
 -struct max77693_led_platform_data {
 - u32 fleds[2];
 +struct max77693_led_config_data {
 + const char *label[2];
   u32 iout_torch[2];
   u32 iout_flash[2];
 - u32 trigger[2];
 - u32 trigger_type[2];
 + u32 flash_timeout[2];
   u32 num_leds;
   u32 boost_mode;
 - u32 flash_timeout;
   u32 boost_vout;
   u32 low_vsys;
 + u32 trigger_type;
  };
  
  /* MAX77693 */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v10 07/19] mfd: max77693: Adjust FLASH_EN_SHIFT and TORCH_EN_SHIFT macros

2015-01-20 Thread Lee Jones
On Tue, 20 Jan 2015, Pavel Machek wrote:
 On Tue 2015-01-20 15:40:29, Lee Jones wrote:
  On Tue, 20 Jan 2015, Jacek Anaszewski wrote:
  
   On 01/20/2015 02:01 PM, Jacek Anaszewski wrote:
   On 01/20/2015 12:17 PM, Lee Jones wrote:
   On Fri, 09 Jan 2015, Jacek Anaszewski wrote:
   
   Modify FLASH_EN_SHIFT and TORCH_EN_SHIFT macros to work properly
   when passed enum max77693_fled values (0 for FLED1 and 1 for FLED2)
   from leds-max77693 driver.
   
   Off-by-one ay?  Wasn't the original code tested?
   
   The driver using these macros is a part of LED / flash API integration
   patch series, which still undergoes modifications and it hasn't
   reached its final state yet, as there are many things to discuss.
   
   To be more precise: the original code had been tested and was working
   properly with the header that is in the mainline. Nonetheless, because
   of the modifications in the driver that was requested during code
   review, it turned out that it would be more convenient to redefine the
   macros.
   
   I'd opt for just agreeing about the mfd related patches and merge
   them no sooner than the leds-max77693 driver is merged.
  
  The only way we can guarantee this is to have them go in during
  different merge-windows, unless of course they go in via the same tree.
 
 Umm. Maintainers should be able to coordinate that. Delaying patch for
 one major release seems rather cruel. Perhaps one maintainer should
 ack the patch and the second one should merge it...

Wow, you're just everywhere today. :)

Read the part after the comma again.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v10 07/19] mfd: max77693: Adjust FLASH_EN_SHIFT and TORCH_EN_SHIFT macros

2015-01-20 Thread Lee Jones
On Tue, 20 Jan 2015, Jacek Anaszewski wrote:

 On 01/20/2015 02:01 PM, Jacek Anaszewski wrote:
 On 01/20/2015 12:17 PM, Lee Jones wrote:
 On Fri, 09 Jan 2015, Jacek Anaszewski wrote:
 
 Modify FLASH_EN_SHIFT and TORCH_EN_SHIFT macros to work properly
 when passed enum max77693_fled values (0 for FLED1 and 1 for FLED2)
 from leds-max77693 driver.
 
 Off-by-one ay?  Wasn't the original code tested?
 
 The driver using these macros is a part of LED / flash API integration
 patch series, which still undergoes modifications and it hasn't
 reached its final state yet, as there are many things to discuss.
 
 To be more precise: the original code had been tested and was working
 properly with the header that is in the mainline. Nonetheless, because
 of the modifications in the driver that was requested during code
 review, it turned out that it would be more convenient to redefine the
 macros.
 
 I'd opt for just agreeing about the mfd related patches and merge
 them no sooner than the leds-max77693 driver is merged.

The only way we can guarantee this is to have them go in during
different merge-windows, unless of course they go in via the same tree.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v10 05/19] mfd: max77693: Modify flash cell name identifiers

2015-01-20 Thread Lee Jones
On Tue, 20 Jan 2015, Jacek Anaszewski wrote:

 On 01/20/2015 12:13 PM, Lee Jones wrote:
 On Fri, 09 Jan 2015, Jacek Anaszewski wrote:
 
 Change flash cell identifiers from max77693-flash to max77693-led
 to avoid confusion with NOR/NAND Flash.
 
 This is okay by me, but aren't these ABI yet?
 
 No, the led driver using it hasn't been merged yet.

Very well.

For my on reference:

Acked-by: Lee Jones lee.jo...@linaro.org

 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
   drivers/mfd/max77693.c |4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
 index a159593..cb14afa 100644
 --- a/drivers/mfd/max77693.c
 +++ b/drivers/mfd/max77693.c
 @@ -53,8 +53,8 @@ static const struct mfd_cell max77693_devs[] = {
 .of_compatible = maxim,max77693-haptic,
 },
 {
 -   .name = max77693-flash,
 -   .of_compatible = maxim,max77693-flash,
 +   .name = max77693-led,
 +   .of_compatible = maxim,max77693-led,
 },
   };
 
 
 
 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v10 09/19] DT: Add documentation for the mfd Maxim max77693

2015-01-20 Thread Lee Jones
On Tue, 20 Jan 2015, Jacek Anaszewski wrote:

 On 01/20/2015 12:21 PM, Lee Jones wrote:
 On Fri, 09 Jan 2015, Jacek Anaszewski wrote:
 
 This patch adds device tree binding documentation for
 the flash cell of the Maxim max77693 multifunctional device.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: Rob Herring robh...@kernel.org
 Cc: Pawel Moll pawel.m...@arm.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Ian Campbell ijc+devicet...@hellion.org.uk
 Cc: Kumar Gala ga...@codeaurora.org
 ---
   Documentation/devicetree/bindings/mfd/max77693.txt |   69 
  
   1 file changed, 69 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt 
 b/Documentation/devicetree/bindings/mfd/max77693.txt
 index 01e9f30..ef184f0 100644
 --- a/Documentation/devicetree/bindings/mfd/max77693.txt
 +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
 @@ -41,7 +41,52 @@ Optional properties:
  To get more informations, please refer to documentaion.
 [*] refer Documentation/devicetree/bindings/pwm/pwm.txt
 
 +- led : the LED submodule device node
 +
 +There are two led outputs available - fled1 and fled2. Each of them can
 +control a separate led or they can be connected together to double
 +the maximum current for a single connected led. One led is represented
 +by one child node.
 +
 +Required properties:
 +- compatible : Must be maxim,max77693-led.
 +
 +Optional properties:
 +- maxim,trigger-type : Flash trigger type.
 +   Possible trigger types:
 +   MAX77693_LED_TRIG_TYPE_EDGE - Rising edge of the signal triggers
 +   the flash,
 +   MAX77693_LED_TRIG_TYPE_LEVEL - Strobe pulse length controls
 +   duration of the flash.
 
 I think you should represent the proper values here instead of the
 defines.
 
 I see both versions in the existing bindings and also a combination of
 them, e.g.: MAX77693_LED_TRIG_TYPE_EDGE (0). I think that it is
 reasonable to mention the macros, especially if they are to appear
 in the DT binding example at the end of the documentation file.
 
 +- maxim,boost-mode :
 +   In boost mode the device can produce up to 1.2A of total current
 +   on both outputs. The maximum current on each output is reduced
 +   to 625mA then. If not enabled explicitly, boost setting defaults to
 +   MAX77693_LED_BOOST_FIXED in case both current sources are used.
 +   Possible values:
 +   MAX77693_LED_BOOST_OFF - no boost,
 +   MAX77693_LED_BOOST_ADAPTIVE - adaptive mode,
 +   MAX77693_LED_BOOST_FIXED - fixed mode.
 
 Same here.
 
 MAX77693_LED_BOOST_OFF (0) - no boost,
 MAX77693_LED_BOOST_ADAPTIVE (1) - adaptive mode,
 MAX77693_LED_BOOST_FIXED (2) - fixed mode.

This is fine too.

 +- maxim,boost-vout : Output voltage of the boost module in millivolts.
 
 -mvout?
 -microvout?
 
 maxim,boost-mvout ?

Right.

 +- maxim,vsys-min : Low input voltage level in millivolts. Flash is not 
 fired
 +   if chip estimates that system voltage could drop below this level due
 +   to flash power consumption.
 
 mvsys?
 microvsys?
 
 maxim,mvsys-min ?

Looks okay to me.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 07/13] mfd: sun6i-prcm: Add support for the ir-clk

2014-12-18 Thread Lee Jones
On Wed, 17 Dec 2014, Hans de Goede wrote:

 Add support for the ir-clk which is part of the sun6i SoC prcm module.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  drivers/mfd/sun6i-prcm.c | 14 ++
  1 file changed, 14 insertions(+)

Pretty standard stuff (

 diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
 index 2f2e9f0..1911731 100644
 --- a/drivers/mfd/sun6i-prcm.c
 +++ b/drivers/mfd/sun6i-prcm.c
 @@ -41,6 +41,14 @@ static const struct resource 
 sun6i_a31_apb0_gates_clk_res[] = {
   },
  };
  
 +static const struct resource sun6i_a31_ir_clk_res[] = {
 + {
 + .start = 0x54,
 + .end = 0x57,
 + .flags = IORESOURCE_MEM,
 + },
 +};

I'm still unkeen on this registers not being defined -- but whateveer!

  static const struct resource sun6i_a31_apb0_rstc_res[] = {
   {
   .start = 0xb0,
 @@ -69,6 +77,12 @@ static const struct mfd_cell sun6i_a31_prcm_subdevs[] = {
   .resources = sun6i_a31_apb0_gates_clk_res,
   },
   {
 + .name = sun6i-a31-ir-clk,
 + .of_compatible = allwinner,sun4i-a10-mod0-clk,
 + .num_resources = ARRAY_SIZE(sun6i_a31_ir_clk_res),
 + .resources = sun6i_a31_ir_clk_res,
 + },
 + {
   .name = sun6i-a31-apb0-clock-reset,
   .of_compatible = allwinner,sun6i-a31-clock-reset,
   .num_resources = ARRAY_SIZE(sun6i_a31_apb0_rstc_res),

This is all pretty standard stuff:

For my own reference:

Acked-by: Lee Jones lee.jo...@linaro.org

Do you do  you expect this patch to be handled?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 07/13] mfd: sun6i-prcm: Add support for the ir-clk

2014-12-18 Thread Lee Jones
On Thu, 18 Dec 2014, Hans de Goede wrote:

 Hi,
 
 On 18-12-14 09:41, Lee Jones wrote:
 On Wed, 17 Dec 2014, Hans de Goede wrote:
 
 Add support for the ir-clk which is part of the sun6i SoC prcm module.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
   drivers/mfd/sun6i-prcm.c | 14 ++
   1 file changed, 14 insertions(+)
 
 Pretty standard stuff (
 
 diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
 index 2f2e9f0..1911731 100644
 --- a/drivers/mfd/sun6i-prcm.c
 +++ b/drivers/mfd/sun6i-prcm.c
 @@ -41,6 +41,14 @@ static const struct resource 
 sun6i_a31_apb0_gates_clk_res[] = {
 },
   };
 
 +static const struct resource sun6i_a31_ir_clk_res[] = {
 +   {
 +   .start = 0x54,
 +   .end = 0x57,
 +   .flags = IORESOURCE_MEM,
 +   },
 +};
 
 I'm still unkeen on this registers not being defined -- but whateveer!
 
   static const struct resource sun6i_a31_apb0_rstc_res[] = {
 {
 .start = 0xb0,
 @@ -69,6 +77,12 @@ static const struct mfd_cell sun6i_a31_prcm_subdevs[] = {
 .resources = sun6i_a31_apb0_gates_clk_res,
 },
 {
 +   .name = sun6i-a31-ir-clk,
 +   .of_compatible = allwinner,sun4i-a10-mod0-clk,
 +   .num_resources = ARRAY_SIZE(sun6i_a31_ir_clk_res),
 +   .resources = sun6i_a31_ir_clk_res,
 +   },
 +   {
 .name = sun6i-a31-apb0-clock-reset,
 .of_compatible = allwinner,sun6i-a31-clock-reset,
 .num_resources = ARRAY_SIZE(sun6i_a31_apb0_rstc_res),
 
 This is all pretty standard stuff:
 
 For my own reference:
 
 Acked-by: Lee Jones lee.jo...@linaro.org
 
 Do you do  you expect this patch to be handled?
 
 I've no preference for how this goes upstream. There are no compile time deps
 and runtime the ir will not work (but not explode) until all the bits are
 in place.

Great, this is my kind of patch.  Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Lee Jones
On Wed, 03 Dec 2014, Jacek Anaszewski wrote:

 Add label array for Device Tree strings with the name of a LED device
 and make flash_timeout a two element array, for caching the sub-led
 related flash timeout. Added is also an array for caching pointers to the
 sub-nodes representing sub-leds.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  include/linux/mfd/max77693.h |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
 index f0b6585..c80ee99 100644
 --- a/include/linux/mfd/max77693.h
 +++ b/include/linux/mfd/max77693.h
 @@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
  };
  
  struct max77693_led_platform_data {
 + const char *label[2];
   u32 fleds[2];
   u32 iout_torch[2];
   u32 iout_flash[2];
   u32 trigger[2];
   u32 trigger_type[2];
 + u32 flash_timeout[2];
   u32 num_leds;
   u32 boost_mode;
 - u32 flash_timeout;
   u32 boost_vout;
   u32 low_vsys;
 + struct device_node *sub_nodes[2];

I haven't seen anyone do this before.  Why can't you use the provided
OF functions to traverse through your tree?

  };
  
  /* MAX77693 */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v9 03/19] mfd: max77693: Modify flash cell name identifiers

2014-12-09 Thread Lee Jones
On Wed, 03 Dec 2014, Jacek Anaszewski wrote:

 Change flash cell identifiers from max77693-flash to max77693-led
 to avoid confusion with NOR/NAND Flash.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  drivers/mfd/max77693.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
 index a159593..cb14afa 100644
 --- a/drivers/mfd/max77693.c
 +++ b/drivers/mfd/max77693.c
 @@ -53,8 +53,8 @@ static const struct mfd_cell max77693_devs[] = {
   .of_compatible = maxim,max77693-haptic,
   },
   {
 - .name = max77693-flash,
 - .of_compatible = maxim,max77693-flash,
 + .name = max77693-led,
 + .of_compatible = maxim,max77693-led,

This is fine by me, so long as you've been through the usual
deprecation procedures or this platform is still WiP.

   },
  };
  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v9 03/19] mfd: max77693: Modify flash cell name identifiers

2014-12-09 Thread Lee Jones
On Tue, 09 Dec 2014, Jacek Anaszewski wrote:

 On 12/09/2014 09:52 AM, Lee Jones wrote:
 On Wed, 03 Dec 2014, Jacek Anaszewski wrote:
 
 Change flash cell identifiers from max77693-flash to max77693-led
 to avoid confusion with NOR/NAND Flash.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
   drivers/mfd/max77693.c |4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
 index a159593..cb14afa 100644
 --- a/drivers/mfd/max77693.c
 +++ b/drivers/mfd/max77693.c
 @@ -53,8 +53,8 @@ static const struct mfd_cell max77693_devs[] = {
 .of_compatible = maxim,max77693-haptic,
 },
 {
 -   .name = max77693-flash,
 -   .of_compatible = maxim,max77693-flash,
 +   .name = max77693-led,
 +   .of_compatible = maxim,max77693-led,
 
 This is fine by me, so long as you've been through the usual
 deprecation procedures or this platform is still WiP.
 
 It was me who added of_compatible for max77693-flash, but the
 related led driver has not been yet merged and there are no
 other drivers depending on it.

Very well.

For my own reference:

Acked-by: Lee Jones lee.jo...@linaro.org

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Lee Jones
On Tue, 09 Dec 2014, Jacek Anaszewski wrote:

 On 12/09/2014 09:50 AM, Lee Jones wrote:
 On Wed, 03 Dec 2014, Jacek Anaszewski wrote:
 
 Add label array for Device Tree strings with the name of a LED device
 and make flash_timeout a two element array, for caching the sub-led
 related flash timeout. Added is also an array for caching pointers to the
 sub-nodes representing sub-leds.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
   include/linux/mfd/max77693.h |4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
 index f0b6585..c80ee99 100644
 --- a/include/linux/mfd/max77693.h
 +++ b/include/linux/mfd/max77693.h
 @@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
   };
 
   struct max77693_led_platform_data {
 +   const char *label[2];
 u32 fleds[2];
 u32 iout_torch[2];for_each_available_child_of_node
 u32 iout_flash[2];
 u32 trigger[2];
 u32 trigger_type[2];
 +   u32 flash_timeout[2];
 u32 num_leds;
 u32 boost_mode;
 -   u32 flash_timeout;
 u32 boost_vout;
 u32 low_vsys;
 +   struct device_node *sub_nodes[2];
 
 I haven't seen anyone do this before.  Why can't you use the provided
 OF functions to traverse through your tree?
 
 I use for_each_available_child_of_node when parsing DT node, but I
 need to cache the pointer to sub-node to be able to use it later
 when it needs to be passed to V4L2 sub-device which is then
 asynchronously matched by the phandle to sub-node.
 
 If it is not well seen to cache it in the platform data then
 I will find different way to accomplish this.

I haven't seen the end-driver for this, but why can't you use that
device's of_node pointer?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Lee Jones
On Tue, 09 Dec 2014, Jacek Anaszewski wrote:

 On 12/09/2014 11:04 AM, Lee Jones wrote:
 On Tue, 09 Dec 2014, Jacek Anaszewski wrote:
 
 On 12/09/2014 09:50 AM, Lee Jones wrote:
 On Wed, 03 Dec 2014, Jacek Anaszewski wrote:
 
 Add label array for Device Tree strings with the name of a LED device
 and make flash_timeout a two element array, for caching the sub-led
 related flash timeout. Added is also an array for caching pointers to the
 sub-nodes representing sub-leds.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
   include/linux/mfd/max77693.h |4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
 index f0b6585..c80ee99 100644
 --- a/include/linux/mfd/max77693.h
 +++ b/include/linux/mfd/max77693.h
 @@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
   };
 
   struct max77693_led_platform_data {
 + const char *label[2];
   u32 fleds[2];
   u32 iout_torch[2];for_each_available_child_of_node
   u32 iout_flash[2];
   u32 trigger[2];
   u32 trigger_type[2];
 + u32 flash_timeout[2];
   u32 num_leds;
   u32 boost_mode;
 - u32 flash_timeout;
   u32 boost_vout;
   u32 low_vsys;
 + struct device_node *sub_nodes[2];
 
 I haven't seen anyone do this before.  Why can't you use the provided
 OF functions to traverse through your tree?
 
 I use for_each_available_child_of_node when parsing DT node, but I
 need to cache the pointer to sub-node to be able to use it later
 when it needs to be passed to V4L2 sub-device which is then
 asynchronously matched by the phandle to sub-node.
 
 If it is not well seen to cache it in the platform data then
 I will find different way to accomplish this.
 
 I haven't seen the end-driver for this, but why can't you use that
 device's of_node pointer?
 
 Maybe it is indeed a good idea. I could pass the of_node pointer
 and the sub-led identifier to the V4L2 sub-device and there look
 for the sub-node containing relevant identifier. The downside
 would be only that for_each_available_child_of_node would
 have to be called twice - in the led driver and in the V4L2 sub-device.
 I think that we can live with it.

Are the LED and V4L2 drivers children of this MFD?  If so, you can use
the of_compatible attribute in struct mfd_cell to populate the each
child's of_node dynamically i.e. the MFD core will do that for you.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v9 04/19] mfd: max77693: adjust max77693_led_platform_data

2014-12-09 Thread Lee Jones
On Tue, 09 Dec 2014, Jacek Anaszewski wrote:

 On 12/09/2014 02:50 PM, Lee Jones wrote:
 On Tue, 09 Dec 2014, Jacek Anaszewski wrote:
 
 On 12/09/2014 11:04 AM, Lee Jones wrote:
 On Tue, 09 Dec 2014, Jacek Anaszewski wrote:
 
 On 12/09/2014 09:50 AM, Lee Jones wrote:
 On Wed, 03 Dec 2014, Jacek Anaszewski wrote:
 
 Add label array for Device Tree strings with the name of a LED device
 and make flash_timeout a two element array, for caching the sub-led
 related flash timeout. Added is also an array for caching pointers to 
 the
 sub-nodes representing sub-leds.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Chanwoo Choi cw00.c...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
   include/linux/mfd/max77693.h |4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
 index f0b6585..c80ee99 100644
 --- a/include/linux/mfd/max77693.h
 +++ b/include/linux/mfd/max77693.h
 @@ -88,16 +88,18 @@ enum max77693_led_boost_mode {
   };
 
   struct max77693_led_platform_data {
 +   const char *label[2];
 u32 fleds[2];
 u32 iout_torch[2];for_each_available_child_of_node
 u32 iout_flash[2];
 u32 trigger[2];
 u32 trigger_type[2];
 +   u32 flash_timeout[2];
 u32 num_leds;
 u32 boost_mode;
 -   u32 flash_timeout;
 u32 boost_vout;
 u32 low_vsys;
 +   struct device_node *sub_nodes[2];
 
 I haven't seen anyone do this before.  Why can't you use the provided
 OF functions to traverse through your tree?
 
 I use for_each_available_child_of_node when parsing DT node, but I
 need to cache the pointer to sub-node to be able to use it later
 when it needs to be passed to V4L2 sub-device which is then
 asynchronously matched by the phandle to sub-node.
 
 If it is not well seen to cache it in the platform data then
 I will find different way to accomplish this.
 
 I haven't seen the end-driver for this, but why can't you use that
 device's of_node pointer?
 
 Maybe it is indeed a good idea. I could pass the of_node pointer
 and the sub-led identifier to the V4L2 sub-device and there look
 for the sub-node containing relevant identifier. The downside
 would be only that for_each_available_child_of_node would
 have to be called twice - in the led driver and in the V4L2 sub-device.
 I think that we can live with it.
 
 Are the LED and V4L2 drivers children of this MFD?  If so, you can use
 the of_compatible attribute in struct mfd_cell to populate the each
 child's of_node dynamically i.e. the MFD core will do that for you.
 
 
 V4L2 driver wraps LED driver. This way the LED device can be
 controlled with use of two interfaces - LED subsystem sysfs
 and V4L2 Flash. This is the aim of the whole patch set.
 
 I've thought it over again and it seems that I will need to cache
 somewhere these sub_nodes pointers. They have to be easily accessible
 for the V4L2 sub-device as it can be asynchronously registered
 or unregistered within V4L2 media device. Sub-devices are matched
 basing on the sub-node phandle.

Not quite getting this.  Can you explain this in another way please?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v8 11/14] DT: Add documentation for the mfd Maxim max77693

2014-12-01 Thread Lee Jones
On Fri, 28 Nov 2014, Jacek Anaszewski wrote:

 This patch adds device tree binding documentation for
 the flash cell of the Maxim max77693 multifunctional device.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: SangYoung Son hello@smasung.com
 Cc: Samuel Ortiz sa...@linux.intel.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: Rob Herring robh...@kernel.org
 Cc: Pawel Moll pawel.m...@arm.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Ian Campbell ijc+devicet...@hellion.org.uk
 Cc: Kumar Gala ga...@codeaurora.org
 Cc: devicet...@vger.kernel.org
 ---
  Documentation/devicetree/bindings/mfd/max77693.txt |   74 
 
  1 file changed, 74 insertions(+)

This definitely requires a DT Ack.

 diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt 
 b/Documentation/devicetree/bindings/mfd/max77693.txt
 index 01e9f30..50a8dad 100644
 --- a/Documentation/devicetree/bindings/mfd/max77693.txt
 +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
 @@ -41,6 +41,62 @@ Optional properties:
To get more informations, please refer to documentaion.
   [*] refer Documentation/devicetree/bindings/pwm/pwm.txt
  
 +- led-flash : the LED submodule device node
 +
 +There are two led outputs available - fled1 and fled2. Each of them can
 +control a separate led or they can be connected together to double
 +the maximum current for a single connected led. One led is represented
 +by one child node.
 +
 +Required properties:
 +- compatible : must be maxim,max77693-flash

I'm not sure this compatible string is suitable.  It looks like
NOR/NAND Flash to me.  Perhaps 'fled', or just 'led' would be better.

 +Optional properties:
 +- maxim,fleds : array of current outputs in order: fled1, fled2

Nit: Sentences start with an uppercase character.

This is true for all other occurrences.

 + Note: both current outputs can be connected to a single led
 + Possible values:
 + 0 - the output is left disconnected,
 + 1 - a diode is connected to the output.
 +- maxim,trigger-type : Array of trigger types in order: flash, torch
 + Possible trigger types:
 + 0 - Rising edge of the signal triggers the flash/torch,
 + 1 - Signal level controls duration of the flash/torch.
 +- maxim,trigger : Array of flags indicating which trigger can activate given 
 led
 + in order: fled1, fled2
 + Possible flag values (can be combined):
 + 1 - FLASH pin of the chip,
 + 2 - TORCH pin of the chip,
 + 4 - software via I2C command.
 +- maxim,boost-mode :
 + In boost mode the device can produce up to 1.2A of total current
 + on both outputs. The maximum current on each output is reduced
 + to 625mA then. If there are two child led nodes defined then boost
 + is enabled by default.
 + Possible values:
 + 0 - no boost,
 + 1 - adaptive mode,
 + 2 - fixed mode.
 +- maxim,boost-vout : Output voltage of the boost module in millivolts.
 +- maxim,vsys-min : Low input voltage level in millivolts. Flash is not fired
 + if chip estimates that system voltage could drop below this level due
 + to flash power consumption.
 +
 +A child node must be defined per sub-led.
 +
 +Required properties of the LED child node:
 +- label : see Documentation/devicetree/bindings/leds/common.txt
 +- maxim,fled_id : identifier of the fled output the led is connected to:
 + 1 - FLED1,
 + 2 - FLED2.

Better to define all of these random numbers in include/dt-bindings.

 +Optional properties of the LED child node:
 +- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 15625 - 25
 +- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
 + Range: 15625 - 100
 +- flash-timeout-microsec : see 
 Documentation/devicetree/bindings/leds/common.txt
 + Range: 62500 - 100
 +
  Example:
   max77693@66 {
   compatible = maxim,max77693;
 @@ -73,4 +129,22 @@ Example:
   pwms = pwm 0 4 0;
   pwm-names = haptic;
   };
 +
 + led_flash: led-flash {

Should both be underscore.  I believe the second portion here should
be more generic led for instance.

 + compatible = maxim,max77693-flash;
 + maxim,fleds = 1 0;
 + maxim,trigger = 7 0;
 + maxim,trigger-type = 0 1;
 + maxim,boost-mode = 0;
 + maxim,boost-vout = 5000;
 + maxim,vsys-min = 2400;

These will all have to be signed off by a DT maintainer.

 + camera-flash {
 + maxim,fled_id = 1

Re: [PATCH/RFC v8 09/14] mfd: max77693: adjust max77693_led_platform_data

2014-12-01 Thread Lee Jones
On Fri, 28 Nov 2014, Jacek Anaszewski wrote:

 Add label array for Device Tree strings with
 the name of a LED device and make flash_timeout
 a two element array, for caching the sub-led
 related flash timeout.

-

Please use all of the 75 char buffer.

 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: SangYoung Son hello@smasung.com
 Cc: Samuel Ortiz sa...@linux.intel.com
 ---
  include/linux/mfd/max77693.h |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
 index f0b6585..30fa19ea 100644
 --- a/include/linux/mfd/max77693.h
 +++ b/include/linux/mfd/max77693.h
 @@ -88,14 +88,15 @@ enum max77693_led_boost_mode {
  };
  
  struct max77693_led_platform_data {
 + const char *label[2];
   u32 fleds[2];
   u32 iout_torch[2];
   u32 iout_flash[2];
   u32 trigger[2];
   u32 trigger_type[2];
 + u32 flash_timeout[2];
   u32 num_leds;
   u32 boost_mode;
 - u32 flash_timeout;
   u32 boost_vout;
   u32 low_vsys;
  };

I'm guessing this will effect the other patches in the set?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v8 09/14] mfd: max77693: adjust max77693_led_platform_data

2014-12-01 Thread Lee Jones
On Mon, 01 Dec 2014, Jacek Anaszewski wrote:
 On 12/01/2014 12:34 PM, Lee Jones wrote:
 On Fri, 28 Nov 2014, Jacek Anaszewski wrote:
 
 I'm guessing this will effect the other patches in the set?
 
 
 max77692 flash driver depends on it and it has to be
 in synch with the related DT bindings patch.

Very well.  Providing you address the commitlog issues:

Acked-by: Lee Jones lee.jo...@linaro.org

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 3/9] clk: sunxi: Add prcm mod0 clock driver

2014-11-27 Thread Lee Jones
On Wed, 26 Nov 2014, Hans de Goede wrote:

 Hi,
 
 On 11/25/2014 05:57 PM, Lee Jones wrote:
 On Sun, 23 Nov 2014, Hans de Goede wrote:
 
 Add a driver for mod0 clocks found in the prcm. Currently there is only
 one mod0 clocks in the prcm, the ir clock.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
   Documentation/devicetree/bindings/clock/sunxi.txt |  1 +
   drivers/clk/sunxi/Makefile|  2 +-
   drivers/clk/sunxi/clk-sun6i-prcm-mod0.c   | 63 
  +++
   drivers/mfd/sun6i-prcm.c  | 14 +
   4 files changed, 79 insertions(+), 1 deletion(-)
   create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm-mod0.c
 
 [...]
 
 diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
 index 283ab8d..ff1254f 100644
 --- a/drivers/mfd/sun6i-prcm.c
 +++ b/drivers/mfd/sun6i-prcm.c
 @@ -41,6 +41,14 @@ static const struct resource 
 sun6i_a31_apb0_gates_clk_res[] = {
 },
   };
 
 +static const struct resource sun6i_a31_ir_clk_res[] = {
 +   {
 +   .start = 0x54,
 +   .end = 0x57,
 +   .flags = IORESOURCE_MEM,
 +   },
 +};
 
 I'm not overly keen on these magic numbers (and yes, I'm well aware
 that I SoB'ed the patch which started them off).
 
 It's not a show stopper, although I'd prefer if they were fixed with a
 subsequent patch.
 
 These are offsets of the relevant registers inside the prcm register block,
 if not done this way, then how should they be done ?

I like these kinds of things to be defined.  No implementation changes
are necessary.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 3/9] clk: sunxi: Add prcm mod0 clock driver

2014-11-25 Thread Lee Jones
On Sun, 23 Nov 2014, Hans de Goede wrote:

 Add a driver for mod0 clocks found in the prcm. Currently there is only
 one mod0 clocks in the prcm, the ir clock.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  Documentation/devicetree/bindings/clock/sunxi.txt |  1 +
  drivers/clk/sunxi/Makefile|  2 +-
  drivers/clk/sunxi/clk-sun6i-prcm-mod0.c   | 63 
 +++
  drivers/mfd/sun6i-prcm.c  | 14 +
  4 files changed, 79 insertions(+), 1 deletion(-)
  create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm-mod0.c

[...]

 diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
 index 283ab8d..ff1254f 100644
 --- a/drivers/mfd/sun6i-prcm.c
 +++ b/drivers/mfd/sun6i-prcm.c
 @@ -41,6 +41,14 @@ static const struct resource 
 sun6i_a31_apb0_gates_clk_res[] = {
   },
  };
  
 +static const struct resource sun6i_a31_ir_clk_res[] = {
 + {
 + .start = 0x54,
 + .end = 0x57,
 + .flags = IORESOURCE_MEM,
 + },
 +};

I'm not overly keen on these magic numbers (and yes, I'm well aware
that I SoB'ed the patch which started them off).

It's not a show stopper, although I'd prefer if they were fixed with a
subsequent patch.

Acked-by: Lee Jones lee.jo...@linaro.org

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v5 2/3] leds: Add support for max77693 mfd flash cell

2014-08-21 Thread Lee Jones
On Wed, 20 Aug 2014, Jacek Anaszewski wrote:

 This patch adds led-flash support to Maxim max77693 chipset.
 A device can be exposed to user space through LED subsystem
 sysfs interface or through V4L2 subdevice when the support
 for V4L2 Flash sub-devices is enabled. Device supports up to
 two leds which can work in flash and torch mode. Leds can
 be triggered externally or by software.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: SangYoung Son hello@smasung.com
 Cc: Samuel Ortiz sa...@linux.intel.com
 ---
  drivers/leds/Kconfig |9 +
  drivers/leds/Makefile|1 +
  drivers/leds/leds-max77693.c | 1048 
 ++
  drivers/mfd/max77693.c   |5 +-
  include/linux/mfd/max77693-private.h |   59 ++
  include/linux/mfd/max77693.h |   40 ++

Please break the MFD changes out into a separate patch, I can't (at
first glace) see any reason why the changed need to be bundled
together like this.

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v5 1/3] mfd: max77693: Fix register enum name

2014-08-21 Thread Lee Jones
On Wed, 20 Aug 2014, Jacek Anaszewski wrote:
 According to the MAX77693 documentation the name of
 the register is FLASH_STATUS.
 
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Lee Jones lee.jo...@linaro.org
 Cc: SangYoung Son hello@smasung.com
 Cc: Samuel Ortiz sa...@linux.intel.com
 ---
  include/linux/mfd/max77693-private.h |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

 diff --git a/include/linux/mfd/max77693-private.h 
 b/include/linux/mfd/max77693-private.h
 index c466ff3..615f121 100644
 --- a/include/linux/mfd/max77693-private.h
 +++ b/include/linux/mfd/max77693-private.h
 @@ -46,7 +46,7 @@ enum max77693_pmic_reg {
   MAX77693_LED_REG_VOUT_FLASH2= 0x0C,
   MAX77693_LED_REG_FLASH_INT  = 0x0E,
   MAX77693_LED_REG_FLASH_INT_MASK = 0x0F,
 - MAX77693_LED_REG_FLASH_INT_STATUS   = 0x10,
 + MAX77693_LED_REG_FLASH_STATUS   = 0x10,
  
   MAX77693_PMIC_REG_PMIC_ID1  = 0x20,
   MAX77693_PMIC_REG_PMIC_ID2  = 0x21,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v3 3/5] leds: Add support for max77693 mfd flash cell

2014-04-16 Thread Lee Jones
 This patch adds led-flash support to Maxim max77693 chipset.
 A device can be exposed to user space through LED subsystem
 sysfs interface or through V4L2 subdevice when the support
 for V4L2 Flash sub-devices is enabled. Device supports up to
 two leds which can work in flash and torch mode. Leds can
 be triggered externally or by software.
 
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: SangYoung Son hello@smasung.com
 Cc: Samuel Ortiz sa...@linux.intel.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  drivers/leds/Kconfig |   10 +
  drivers/leds/Makefile|1 +
  drivers/leds/leds-max77693.c |  794 
 ++
  drivers/mfd/max77693.c   |2 +-
  include/linux/mfd/max77693.h |   38 ++
  5 files changed, 844 insertions(+), 1 deletion(-)
  create mode 100644 drivers/leds/leds-max77693.c

[...]

 diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
 index c5535f0..f061aa8 100644
 --- a/drivers/mfd/max77693.c
 +++ b/drivers/mfd/max77693.c
 @@ -44,7 +44,7 @@
  static const struct mfd_cell max77693_devs[] = {
   { .name = max77693-pmic, },
   { .name = max77693-charger, },
 - { .name = max77693-flash, },
 + { .name = max77693-flash, .of_compatible = maxim,max77693-flash, },

I would prefer for this to be opened up i.e. not on one line.

   { .name = max77693-muic, },
   { .name = max77693-haptic, },
  };
 diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
 index 3f3dc45..f2285b7 100644
 --- a/include/linux/mfd/max77693.h
 +++ b/include/linux/mfd/max77693.h
 @@ -63,6 +63,43 @@ struct max77693_muic_platform_data {
   int path_uart;
  };
  
 +/* MAX77693 led flash */
 +
 +/* triggers */
 +enum max77693_led_trigger {
 + MAX77693_LED_TRIG_OFF,
 + MAX77693_LED_TRIG_FLASH,
 + MAX77693_LED_TRIG_TORCH,
 + MAX77693_LED_TRIG_EXT,
 + MAX77693_LED_TRIG_SOFT,
 +};
 +
 +

Extra '\n' here.

 +/* trigger types */
 +enum max77693_led_trigger_type {
 + MAX77693_LED_TRIG_TYPE_EDGE,
 + MAX77693_LED_TRIG_TYPE_LEVEL,
 +};
 +
 +/* boost modes */
 +enum max77693_led_boost_mode {
 + MAX77693_LED_BOOST_NONE,
 + MAX77693_LED_BOOST_ADAPTIVE,
 + MAX77693_LED_BOOST_FIXED,
 +};
 +
 +struct max77693_led_platform_data {
 + u32 iout[4];
 + u32 trigger[4];
 + u32 trigger_type[2];
 + u32 timeout[2];
 + u32 boost_mode[2];
 + u32 boost_vout;
 + u32 low_vsys;
 +};

Bryan will have to review this.

 +/* MAX77693 */
 +
  struct max77693_platform_data {
   /* regulator data */
   struct max77693_regulator_data *regulators;
 @@ -70,5 +107,6 @@ struct max77693_platform_data {
  
   /* muic data */
   struct max77693_muic_platform_data *muic_data;
 + struct max77693_led_platform_data *led_data;
  };
  #endif   /* __LINUX_MFD_MAX77693_H */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v2 4/8] leds: Add support for max77693 mfd flash cell

2014-03-31 Thread Lee Jones
On Fri, 28 Mar 2014, Jacek Anaszewski wrote:

 This patch adds led-flash support to Maxim max77693 chipset.
 Device can be exposed to user space through LED subsystem
 sysfs interface or through V4L2 subdevice when the support
 for Multimedia Framework is enabled. Device supports up to
 two leds which can work in flash and torch mode. Leds can
 be triggered externally or by software.
 
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: SangYoung Son hello@smasung.com
 Cc: Samuel Ortiz sa...@linux.intel.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  drivers/leds/Kconfig |   10 +
  drivers/leds/Makefile|1 +
  drivers/leds/leds-max77693.c |  864 
 ++
  drivers/mfd/max77693.c   |3 +-
  include/linux/mfd/max77693.h |   32 ++
  5 files changed, 909 insertions(+), 1 deletion(-)
  create mode 100644 drivers/leds/leds-max77693.c

[...]

 diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
 index c5535f0..d53c497 100644
 --- a/drivers/mfd/max77693.c
 +++ b/drivers/mfd/max77693.c
 @@ -44,7 +44,8 @@
  static const struct mfd_cell max77693_devs[] = {
   { .name = max77693-pmic, },
   { .name = max77693-charger, },
 - { .name = max77693-flash, },
 + { .name = max77693-flash,
 +   .of_compatible = maxim,max77693-flash, },

On one line please.

   { .name = max77693-muic, },
   { .name = max77693-haptic, },
  };
 diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
 index 3f3dc45..5859698 100644
 --- a/include/linux/mfd/max77693.h
 +++ b/include/linux/mfd/max77693.h
 @@ -63,6 +63,37 @@ struct max77693_muic_platform_data {
   int path_uart;
  };
  
 +/* MAX77693 led flash */
 +
 +/* triggers */
 +#define MAX77693_LED_TRIG_OFF0
 +#define MAX77693_LED_TRIG_FLASH  1
 +#define MAX77693_LED_TRIG_TORCH  2
 +#define MAX77693_LED_TRIG_EXT(MAX77693_LED_TRIG_FLASH |\
 + MAX77693_LED_TRIG_TORCH)
 +#define MAX77693_LED_TRIG_SOFT   4
 +
 +/* trigger types */
 +#define MAX77693_LED_TRIG_TYPE_EDGE  0
 +#define MAX77693_LED_TRIG_TYPE_LEVEL 1
 +
 +/* boost modes */
 +#define MAX77693_LED_BOOST_NONE  0
 +#define MAX77693_LED_BOOST_ADAPTIVE  1
 +#define MAX77693_LED_BOOST_FIXED 2

I think it would be better to enum all of the above.

 +struct max77693_led_platform_data {
 + u32 iout[4];
 + u32 trigger[4];
 + u32 trigger_type[2];
 + u32 timeout[2];
 + u32 boost_mode[2];
 + u32 boost_vout;
 + u32 low_vsys;
 +};

I'll leave this LED stuff to the expert(s).

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 6/8] leds: Add support for max77693 mfd flash cell

2014-03-21 Thread Lee Jones
 This patch adds led-flash support to Maxim max77693 chipset.
 Device can be exposed to user space through LED subsystem
 sysfs interface or through V4L2 subdevice when the support
 for Multimedia Framework is enabled. Device supports up to
 two leds which can work in flash and torch mode. Leds can
 be triggered externally or by software.
 
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: SangYoung Son hello@smasung.com
 Cc: Samuel Ortiz sa...@linux.intel.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
   drivers/leds/Kconfig |9 +
   drivers/leds/Makefile|1 +
   drivers/leds/leds-max77693.c |  768 
  ++
   drivers/mfd/max77693.c   |   21 +-
   include/linux/mfd/max77693.h |   32 ++
   5 files changed, 825 insertions(+), 6 deletions(-)
   create mode 100644 drivers/leds/leds-max77693.c
 
 [...]
 -static const struct mfd_cell max77693_devs[] = {
 -   { .name = max77693-pmic, },
 -   { .name = max77693-charger, },
 -   { .name = max77693-flash, },
 -   { .name = max77693-muic, },
 -   { .name = max77693-haptic, },
 +enum mfd_devs_idx {
 +   IDX_PMIC,
 +   IDX_CHARGER,
 +   IDX_LED,
 +   IDX_MUIC,
 +   IDX_HAPTIC,
 +};
 +
 +static struct mfd_cell max77693_devs[] = {
 +   [IDX_PMIC]  = { .name = max77693-pmic, },
 +   [IDX_CHARGER]   = { .name = max77693-charger, },
 +   [IDX_LED]   = { .name = max77693-led,
 +   .of_compatible = maxim,max77693-led},
 +   [IDX_MUIC]  = { .name = max77693-muic, },
 +   [IDX_HAPTIC]= { .name = max77693-haptic, },
   };
 
 What is the purpose of this change?
 
 Introducing mfd_devs_idx itself is a cosmetic change, which
 actually could be avoided. Initialization of the of_compatible field
 is required for the led driver to get matched properly. And as I've
 just realized also max77693-flash name should be preserved.
 I will fix this in the next version of the patch.

I'm happy with the addition of any .of_compatible strings, however
please leave out the IDXs in your next version(s).

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 6/8] leds: Add support for max77693 mfd flash cell

2014-03-20 Thread Lee Jones
On Thu, 20 Mar 2014, Jacek Anaszewski wrote:

 This patch adds led-flash support to Maxim max77693 chipset.
 Device can be exposed to user space through LED subsystem
 sysfs interface or through V4L2 subdevice when the support
 for Multimedia Framework is enabled. Device supports up to
 two leds which can work in flash and torch mode. Leds can
 be triggered externally or by software.
 
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Bryan Wu coolo...@gmail.com
 Cc: Richard Purdie rpur...@rpsys.net
 Cc: SangYoung Son hello@smasung.com
 Cc: Samuel Ortiz sa...@linux.intel.com
 Cc: Lee Jones lee.jo...@linaro.org
 ---
  drivers/leds/Kconfig |9 +
  drivers/leds/Makefile|1 +
  drivers/leds/leds-max77693.c |  768 
 ++
  drivers/mfd/max77693.c   |   21 +-
  include/linux/mfd/max77693.h |   32 ++
  5 files changed, 825 insertions(+), 6 deletions(-)
  create mode 100644 drivers/leds/leds-max77693.c

[...]

 diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
 index c5535f0..6fa92d3 100644
 --- a/drivers/mfd/max77693.c
 +++ b/drivers/mfd/max77693.c
 @@ -41,12 +41,21 @@
  #define I2C_ADDR_MUIC(0x4A  1)
  #define I2C_ADDR_HAPTIC  (0x90  1)
  
 -static const struct mfd_cell max77693_devs[] = {
 - { .name = max77693-pmic, },
 - { .name = max77693-charger, },
 - { .name = max77693-flash, },
 - { .name = max77693-muic, },
 - { .name = max77693-haptic, },
 +enum mfd_devs_idx {
 + IDX_PMIC,
 + IDX_CHARGER,
 + IDX_LED,
 + IDX_MUIC,
 + IDX_HAPTIC,
 +};
 +
 +static struct mfd_cell max77693_devs[] = {
 + [IDX_PMIC]  = { .name = max77693-pmic, },
 + [IDX_CHARGER]   = { .name = max77693-charger, },
 + [IDX_LED]   = { .name = max77693-led,
 + .of_compatible = maxim,max77693-led},
 + [IDX_MUIC]  = { .name = max77693-muic, },
 + [IDX_HAPTIC]= { .name = max77693-haptic, },
  };

What is the purpose of this change?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC v2 1/2] max77693: added device tree support

2013-08-14 Thread Lee Jones
 I have no response from Samuel regarding this patch.
 Could you take care of it, I can rebase it again if necessary.

Yes, please rebase onto v3.11-rc5 and resubmit.

Thanks.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html