Re: [PATCH v7 0/6] Add ChromeOS EC CEC Support
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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)
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
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
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
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
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.
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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