* Re: [PATCH v5] HID: intel-ish-hid: ISH firmware loader client driver
From: Rushikesh S Kadam @ 2019-04-03 13:35 UTC (permalink / raw)
To: Jiri Kosina
Cc: srinivas.pandruvada, benjamin.tissoires, ncrews, jettrink,
gwendal, linux-kernel, linux-input
In-Reply-To: <nycvar.YFH.7.76.1904031510430.9803@cbobk.fhfr.pm>
On Wed, Apr 03, 2019 at 03:11:02PM +0200, Jiri Kosina wrote:
> On Tue, 2 Apr 2019, Rushikesh S Kadam wrote:
>
> > This driver adds support for loading Intel Integrated
> > Sensor Hub (ISH) firmware from host file system to ISH
> > SRAM and start execution.
> >
> > At power-on, the ISH subsystem shall boot to an interim
> > Shim loader-firmware, which shall expose an ISHTP loader
> > device.
> >
> > The driver implements an ISHTP client that communicates
> > with the Shim ISHTP loader device over the intel-ish-hid
> > stack, to download the main ISH firmware.
> >
> > Signed-off-by: Rushikesh S Kadam <rushikesh.s.kadam@intel.com>
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Acked-by: Nick Crews <ncrews@chromium.org>
> > Tested-by: Jett Rink <jettrink@chromium.org>
>
> Now queued in for-5.2/ish. Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
Thanks Jirk & all for help with this.
Rushikesh
--
^ permalink raw reply
* Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs
From: Aaron Ma @ 2019-04-03 13:58 UTC (permalink / raw)
To: Christopher Heiny, dmitry.torokhov@gmail.com,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Andrew Duggan, benjamin.tissoires@redhat.com
In-Reply-To: <f4f7265d9ad06726bb2ef12ab6b7529d2fb06165.camel@synaptics.com>
On 4/3/19 12:16 AM, Christopher Heiny wrote:
> On Thu, 2019-03-28 at 14:02 +0800, Aaron Ma wrote:
>> Hi Dmitry and Chiristopher:
>>
>> Do you have any suggestion about these 2 patches?
>>
>> Many users confirmed that they fixed issues of Trackpoint/Touchpad
>> after S3.
>>
>> Will you consider them be accepted?
> Hi Aaron,
>
> Sorry - I thought I'd replied on the NO SLEEP portion of these patches,
> but looking back I don't find the draft or the sent email. Sorry about
> that. I'll summarize here what I wrote last month.
>
> This isn't so much a "fix" as a "hacky workaround" for the issue. From
> the descriptions in the bug you linked in your original patch
> submission, it appears that the root cause is somewhere in SMBus system
> (could be SMBus driver, SMBus hardware, or the devices on the SMBus
> (touch devices or other devices) - it's hard to tell with the info
> available), where the SMBus is failing to come online correctly coming
> out of S3 state. Anyway, this patch doesn't fix the root cause, but
> merely works around it.
Users confirmed the 1st patch that clear irq status fixed their multiple
issues on Touchpad and Trackpoint.
I think it is a fix.
NO SLEEP patch was tried to give users a choice a fix touchpad issues
that I didn't reproduce.
If you don't like this export, we can drop it now as users confirmed 1st
patch works.
>
> Setting the NO SLEEP bit will force the touch sensor to remain in a
> high power consumption state while the rest of the system is in S3.
> While not a lot of power compared to things like the CPU, display, and
> others, it is still non-trivial and will result in shorter time-on-
> battery capability.
Verified on s2idle and S3 and system running idle mode. no difference on
power consumption of whole system with or without set 1 to nosleep.
>
> If you have access to the power pin(s) for the touch sensor(s)/styk(s),
> it might be interesting to try turning power off on entering S3, and
> restoring it on exit. That's very hacky, and has the side effect of
> slightly delaying touchpad readiness on exit from S3. Plus you'll need
> to restore touch sensor configuration settings on exit. But it
> definitely reduces power consumption.
>
>
> Separately, I am still concerned about the possibility of dropped touch
> events in the IRQ clearing. I'm not convinced that the code is safe
> (as you mentioned in your reply to my earlier comment), so I'll have to
> study the implementation more carefully.
Sure, take your time, if you have any questions let me know please.
Thanks,
Aaron
>
> Cheers,
> Chris
>
>
>
>> Thanks,
>> Aaron
>
^ permalink raw reply
* [BUG REPORT] linux-input: keyboard: gpio_keys: False Button Press Event on Wake
From: Ken Sloat @ 2019-04-03 17:50 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com
Cc: josephl@nvidia.com, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Ken Sloat
Hello Dmitry,
I may have found a potential bug in the "gpio_keys" driver. FYI, I am running the 4.14 LTS kernel on my system, but from my understanding of the issue, it seems that this would still occur in the latest version of the kernel.
The problem:
In the 4.14 LTS kernel, both key press and release events can generate a wake event. In the 5.x kernel, wake events are configurable for press only, release only or "both" (see "wakeup-event-action" binding). The issue can occur in the "both" case or release/deasserted case. Let's imagine that a system is suspended when a gpio key button is pressed, and subsequently resumed when the button is released. If we look at the sequence of actions and events reported by the input system, we can see the potential problem:
Button Pressed
Event Value 1
System Suspend
Button Released
System Wake & Resume
Event Value 0
Event Value 1
Event Value 0
As you can see the input system will report an extra button event/press. This appears to be caused in gpio_keys_gpio_isr by the following statement:
if (bdata->suspended &&
(button->type == 0 || button->type == EV_KEY)) {
/*
* Simulate wakeup key press in case the key has
* already released by the time we got interrupt
* handler to run.
*/
input_report_key(bdata->input, button->code, 1);
}
This code does not seem to take into account that the wake event may have been caused by a button release action, and just assumes we must have a button press.
This can obviously be problematic in the use case I mentioned, as the system would be put in a constant loop between waking and sleeping. While there are other ways to deal with or react to this issue in the userspace, it seems that the driver should probably take this into account.
Thoughts?
Thanks,
Ken Sloat
^ permalink raw reply
* Re: [PATCH] Input: evdev - use struct_size() in kzalloc() and vzalloc()
From: Dmitry Torokhov @ 2019-04-03 17:51 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: linux-input, linux-kernel
In-Reply-To: <20190221153738.GA17825@embeddedor>
On Thu, Feb 21, 2019 at 09:37:38AM -0600, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> size = sizeof(struct foo) + count * sizeof(struct boo);
> instance = kzalloc(size, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> Notice that, in this case, variable size is not necessary, hence
> it is removed.
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied, thank you.
> ---
> drivers/input/evdev.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index f48369d6f3a0..ee8dd8b1b09e 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -503,14 +503,13 @@ static int evdev_open(struct inode *inode, struct file *file)
> {
> struct evdev *evdev = container_of(inode->i_cdev, struct evdev, cdev);
> unsigned int bufsize = evdev_compute_buffer_size(evdev->handle.dev);
> - unsigned int size = sizeof(struct evdev_client) +
> - bufsize * sizeof(struct input_event);
> struct evdev_client *client;
> int error;
>
> - client = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
> + client = kzalloc(struct_size(client, buffer, bufsize),
> + GFP_KERNEL | __GFP_NOWARN);
> if (!client)
> - client = vzalloc(size);
> + client = vzalloc(struct_size(client, buffer, bufsize));
> if (!client)
> return -ENOMEM;
>
> --
> 2.20.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/3] Input: lpc32xx-key - add clocks property and fix DT binding example
From: Dmitry Torokhov @ 2019-04-03 17:53 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Rob Herring, devicetree, linux-input, linux-arm-kernel,
Sylvain Lemieux
In-Reply-To: <41befcd7-5ca4-34c9-6929-1ae1d706b108@mleia.com>
On Sat, Feb 23, 2019 at 01:38:01PM +0200, Vladimir Zapolskiy wrote:
> Hi Dmitry,
>
> On 02/23/2019 02:41 AM, Rob Herring wrote:
> > On Sat, 26 Jan 2019 16:29:19 +0200, Vladimir Zapolskiy wrote:
> >> The keypad controller on NXP LPC32xx requires its clock gate to be open,
> >> therefore add description of the requires 'clocks' property.
> >>
> >> In addition adjust the example by adding description of required 'clocks'
> >> property and by fixing 'interrupts' property.
> >>
> >> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> >> ---
> >> Documentation/devicetree/bindings/input/lpc32xx-key.txt | 5 ++++-
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> >
>
> can you please pull this documentation change through Linux input branch?
>
> The two other dts changes have been already included into arm-soc.
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [BUG REPORT] linux-input: keyboard: gpio_keys: False Button Press Event on Wake
From: dmitry.torokhov @ 2019-04-03 18:16 UTC (permalink / raw)
To: Ken Sloat
Cc: josephl@nvidia.com, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <BL0PR07MB41151EC4B770F6A7396C152EAD570@BL0PR07MB4115.namprd07.prod.outlook.com>
Hi Ken,
On Wed, Apr 03, 2019 at 05:50:09PM +0000, Ken Sloat wrote:
> Hello Dmitry,
>
> I may have found a potential bug in the "gpio_keys" driver. FYI, I am
> running the 4.14 LTS kernel on my system, but from my understanding of
> the issue, it seems that this would still occur in the latest version
> of the kernel.
>
> The problem: In the 4.14 LTS kernel, both key press and release events
> can generate a wake event. In the 5.x kernel, wake events are
> configurable for press only, release only or "both" (see
> "wakeup-event-action" binding). The issue can occur in the "both" case
> or release/deasserted case. Let's imagine that a system is suspended
> when a gpio key button is pressed, and subsequently resumed when the
> button is released. If we look at the sequence of actions and events
> reported by the input system, we can see the potential problem:
>
> Button Pressed
> Event Value 1
> System Suspend
> Button Released
> System Wake & Resume
> Event Value 0
> Event Value 1
> Event Value 0
>
> As you can see the input system will report an extra button
> event/press. This appears to be caused in gpio_keys_gpio_isr by the
> following statement:
>
> if (bdata->suspended &&
> (button->type == 0 || button->type == EV_KEY)) {
> /*
> * Simulate wakeup key press in case the key has
> * already released by the time we got interrupt
> * handler to run.
> */
> input_report_key(bdata->input, button->code, 1);
> }
>
> This code does not seem to take into account that the wake event may
> have been caused by a button release action, and just assumes we must
> have a button press.
>
> This can obviously be problematic in the use case I mentioned, as the
> system would be put in a constant loop between waking and sleeping.
> While there are other ways to deal with or react to this issue in the
> userspace, it seems that the driver should probably take this into
> account.
>
I believe the expectation is that we do not go to sleep with button
still pressed, as we expect it to be released momentarily. Given that we
do not know which edge woke us it is not clear if we can avoid
simulating the keypress event, as this definitely causes "missed press",
at least on some Android devices, where by the time we get to run this
ISR user has already released the button.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
From: dmitry.torokhov @ 2019-04-03 21:48 UTC (permalink / raw)
To: Anson Huang
Cc: shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, dl-linux-imx
In-Reply-To: <1553842562-8481-1-git-send-email-Anson.Huang@nxp.com>
Hi Anson,
On Fri, Mar 29, 2019 at 07:00:43AM +0000, Anson Huang wrote:
> There is no need to enable release interrupt and disable depress
> interrupt in event check, as a timer is setup for checking these
> events rather than interrupts.
But won't using release interrupt allow signalling key release earlier?
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> drivers/input/keyboard/imx_keypad.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
> index 539cb67..7e32c36 100644
> --- a/drivers/input/keyboard/imx_keypad.c
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct timer_list *t)
> reg_val = readw(keypad->mmio_base + KPSR);
> reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
> writew(reg_val, keypad->mmio_base + KPSR);
> -
> - reg_val = readw(keypad->mmio_base + KPSR);
> - reg_val |= KBD_STAT_KRIE;
> - reg_val &= ~KBD_STAT_KDIE;
> - writew(reg_val, keypad->mmio_base + KPSR);
> }
> }
>
> --
> 2.7.4
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH V2] input: keyboard: snvs: initialize necessary driver data before enabling IRQ
From: dmitry.torokhov @ 2019-04-03 22:15 UTC (permalink / raw)
To: Anson Huang
Cc: Fabio Estevam, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx
In-Reply-To: <1553666514-21505-1-git-send-email-Anson.Huang@nxp.com>
On Wed, Mar 27, 2019 at 06:07:06AM +0000, Anson Huang wrote:
> SNVS IRQ is requested before necessary driver data initialized,
> if there is a pending IRQ during driver probe phase, kernel
> NULL pointer panic will occur in IRQ handler. To avoid such
> scenario, just initialize necessary driver data before enabling
> IRQ. This patch is inspired by NXP's internal kernel tree.
>
> Fixes: d3dc6e232215 ("input: keyboard: imx: add snvs power key driver")
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Applied, thank you.
> ---
> Changes since V1:
> - move the platform data initialization to before IRQ enable instead of moving the IRQ enable
> to the end of probe function.
> ---
> drivers/input/keyboard/snvs_pwrkey.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index effb632..4c67cf3 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -148,6 +148,9 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> return error;
> }
>
> + pdata->input = input;
> + platform_set_drvdata(pdev, pdata);
> +
> error = devm_request_irq(&pdev->dev, pdata->irq,
> imx_snvs_pwrkey_interrupt,
> 0, pdev->name, pdev);
> @@ -163,9 +166,6 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> return error;
> }
>
> - pdata->input = input;
> - platform_set_drvdata(pdev, pdata);
> -
> device_init_wakeup(&pdev->dev, pdata->wakeup);
>
> return 0;
> --
> 2.7.4
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: keyboard: snvs: use dev_pm_set_wake_irq() to simplify code
From: dmitry.torokhov @ 2019-04-03 22:16 UTC (permalink / raw)
To: Anson Huang
Cc: Fabio Estevam, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx
In-Reply-To: <1553666576-21621-1-git-send-email-Anson.Huang@nxp.com>
On Wed, Mar 27, 2019 at 06:08:05AM +0000, Anson Huang wrote:
> With calling dev_pm_set_wake_irq() to set SNVS ON/OFF button
> as wakeup source for suspend, generic wake irq mechanism
> will automatically enable it as wakeup source when suspend,
> then the enable_irq_wake()/disable_irq_wake() can be removed
> in suspend/resume callback, it simplifies the code.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Applied, thank you.
> ---
> drivers/input/keyboard/snvs_pwrkey.c | 30 ++++--------------------------
> 1 file changed, 4 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index 4c67cf3..5342d8d 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -15,6 +15,7 @@
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_wakeirq.h>
> #include <linux/mfd/syscon.h>
> #include <linux/regmap.h>
>
> @@ -167,28 +168,9 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
> }
>
> device_init_wakeup(&pdev->dev, pdata->wakeup);
> -
> - return 0;
> -}
> -
> -static int __maybe_unused imx_snvs_pwrkey_suspend(struct device *dev)
> -{
> - struct platform_device *pdev = to_platform_device(dev);
> - struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
> -
> - if (device_may_wakeup(&pdev->dev))
> - enable_irq_wake(pdata->irq);
> -
> - return 0;
> -}
> -
> -static int __maybe_unused imx_snvs_pwrkey_resume(struct device *dev)
> -{
> - struct platform_device *pdev = to_platform_device(dev);
> - struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
> -
> - if (device_may_wakeup(&pdev->dev))
> - disable_irq_wake(pdata->irq);
> + error = dev_pm_set_wake_irq(&pdev->dev, pdata->irq);
> + if (error)
> + dev_err(&pdev->dev, "irq wake enable failed.\n");
>
> return 0;
> }
> @@ -199,13 +181,9 @@ static const struct of_device_id imx_snvs_pwrkey_ids[] = {
> };
> MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
>
> -static SIMPLE_DEV_PM_OPS(imx_snvs_pwrkey_pm_ops, imx_snvs_pwrkey_suspend,
> - imx_snvs_pwrkey_resume);
> -
> static struct platform_driver imx_snvs_pwrkey_driver = {
> .driver = {
> .name = "snvs_pwrkey",
> - .pm = &imx_snvs_pwrkey_pm_ops,
> .of_match_table = imx_snvs_pwrkey_ids,
> },
> .probe = imx_snvs_pwrkey_probe,
> --
> 2.7.4
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v7 2/4] Input: goodix - Add regulators suppot
From: Dmitry Torokhov @ 2019-04-03 23:09 UTC (permalink / raw)
To: Jagan Teki
Cc: Bastien Nocera, Rob Herring, Henrik Rydberg, linux-input,
linux-kernel, devicetree, Mark Rutland, linux-amarula,
Michael Trimarchi
In-Reply-To: <20190321082104.2874-3-jagan@amarulasolutions.com>
Hi Jagan,
On Thu, Mar 21, 2019 at 01:51:02PM +0530, Jagan Teki wrote:
> Goodix CTP controllers require AVDD28, VDDIO regulators for power-on
> sequence.
>
> The delay between these regualtor operations as per Power-on Timing
> from datasheet[1] is 0 (T1 >= 0 usec).
>
> So, enable and disable these regulators in proper order using normal
> regulator functions without any delay in between.
>
> [1] GT5663 Datasheet_English_20151106_Rev.01
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> drivers/input/touchscreen/goodix.c | 58 ++++++++++++++++++++++++++++++
> 1 file changed, 58 insertions(+)
>
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index f57d82220a88..de5b80a08f41 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -27,6 +27,7 @@
> #include <linux/delay.h>
> #include <linux/irq.h>
> #include <linux/interrupt.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> #include <linux/acpi.h>
> #include <linux/of.h>
> @@ -47,6 +48,8 @@ struct goodix_ts_data {
> struct touchscreen_properties prop;
> unsigned int max_touch_num;
> unsigned int int_trigger_type;
> + struct regulator *avdd28;
> + struct regulator *vddio;
> struct gpio_desc *gpiod_int;
> struct gpio_desc *gpiod_rst;
> u16 id;
> @@ -532,6 +535,24 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts)
> return -EINVAL;
> dev = &ts->client->dev;
>
> + ts->avdd28 = devm_regulator_get(dev, "AVDD28");
> + if (IS_ERR(ts->avdd28)) {
> + error = PTR_ERR(ts->avdd28);
> + if (error != -EPROBE_DEFER)
> + dev_err(dev,
> + "Failed to get AVDD28 regulator: %d\n", error);
> + return error;
> + }
> +
> + ts->vddio = devm_regulator_get(dev, "VDDIO");
> + if (IS_ERR(ts->vddio)) {
> + error = PTR_ERR(ts->vddio);
> + if (error != -EPROBE_DEFER)
> + dev_err(dev,
> + "Failed to get VDDIO regulator: %d\n", error);
> + return error;
> + }
> +
> /* Get the interrupt GPIO pin number */
> gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_INT_NAME, GPIOD_IN);
> if (IS_ERR(gpiod)) {
> @@ -764,6 +785,17 @@ static void goodix_config_cb(const struct firmware *cfg, void *ctx)
> complete_all(&ts->firmware_loading_complete);
> }
>
> +static void goodix_disable_regulator(void *arg)
> +{
> + struct goodix_ts_data *ts = arg;
> +
> + if (!IS_ERR(ts->vddio))
> + regulator_disable(ts->vddio);
We error out of goodix_get_gpio_config() and abort probe() if
devm_regulator_get() fails, so there is no need to check for errors
here.
> +
> + if (!IS_ERR(ts->avdd28))
> + regulator_disable(ts->avdd28);
> +}
> +
> static int goodix_ts_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -789,6 +821,32 @@ static int goodix_ts_probe(struct i2c_client *client,
> if (error)
> return error;
>
> + error = devm_add_action_or_reset(&client->dev,
> + goodix_disable_regulator, ts);
> + if (error)
> + return error;
We need to do this after enabling regulators, otherwise there is a risk
of unbalanced disable.
I adjusted and applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v7 4/4] Input: goodix - Add GT5663 CTP support
From: Dmitry Torokhov @ 2019-04-03 23:09 UTC (permalink / raw)
To: Jagan Teki
Cc: Bastien Nocera, Rob Herring, Henrik Rydberg, linux-input,
linux-kernel, devicetree, Mark Rutland, linux-amarula,
Michael Trimarchi
In-Reply-To: <20190321082104.2874-5-jagan@amarulasolutions.com>
On Thu, Mar 21, 2019 at 01:51:04PM +0530, Jagan Teki wrote:
> GT5663 is capacitive touch controller with customized smart
> wakeup gestures.
>
> Add support for it by adding compatible and supported chip data.
>
> The chip data on GT5663 is similar to GT1151, like
> - config data register has 0x8050 address
> - config data register max len is 240
> - config data checksum has 16-bit
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/goodix.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index de5b80a08f41..c558b091749c 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -219,6 +219,7 @@ static const struct goodix_chip_data *goodix_get_chip_data(u16 id)
> {
> switch (id) {
> case 1151:
> + case 5663:
> case 5688:
> return >1x_chip_data;
>
> @@ -1003,6 +1004,7 @@ MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
> #ifdef CONFIG_OF
> static const struct of_device_id goodix_of_match[] = {
> { .compatible = "goodix,gt1151" },
> + { .compatible = "goodix,gt5663" },
> { .compatible = "goodix,gt5688" },
> { .compatible = "goodix,gt911" },
> { .compatible = "goodix,gt9110" },
> --
> 2.18.0.321.gffc6fa0e3
>
--
Dmitry
^ permalink raw reply
* RE: [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
From: Anson Huang @ 2019-04-04 1:29 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com
Cc: shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, dl-linux-imx
In-Reply-To: <20190403214842.GD53104@dtor-ws>
Hi, Dmitry
Best Regards!
Anson Huang
> -----Original Message-----
> From: dmitry.torokhov@gmail.com [mailto:dmitry.torokhov@gmail.com]
> Sent: 2019年4月4日 5:49
> To: Anson Huang <anson.huang@nxp.com>
> Cc: shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-input@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-
> imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/2] input: keyboard: imx: no need to control interrupt
> status in event check
>
> Hi Anson,
>
> On Fri, Mar 29, 2019 at 07:00:43AM +0000, Anson Huang wrote:
> > There is no need to enable release interrupt and disable depress
> > interrupt in event check, as a timer is setup for checking these
> > events rather than interrupts.
>
> But won't using release interrupt allow signalling key release earlier?
It makes sense, patch #1 can be dropped, I will resend the patch #2 for review.
Thanks,
Anson.
>
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > drivers/input/keyboard/imx_keypad.c | 5 -----
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/imx_keypad.c
> > b/drivers/input/keyboard/imx_keypad.c
> > index 539cb67..7e32c36 100644
> > --- a/drivers/input/keyboard/imx_keypad.c
> > +++ b/drivers/input/keyboard/imx_keypad.c
> > @@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct
> timer_list *t)
> > reg_val = readw(keypad->mmio_base + KPSR);
> > reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
> > writew(reg_val, keypad->mmio_base + KPSR);
> > -
> > - reg_val = readw(keypad->mmio_base + KPSR);
> > - reg_val |= KBD_STAT_KRIE;
> > - reg_val &= ~KBD_STAT_KDIE;
> > - writew(reg_val, keypad->mmio_base + KPSR);
> > }
> > }
> >
> > --
> > 2.7.4
> >
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* [RESEND] input: keyboard: imx: make sure keyboard can always wake up system
From: Anson Huang @ 2019-04-04 1:40 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: dl-linux-imx
There are several scenarios that keyboard can NOT wake up system
from suspend, e.g., if a keyboard is depressed between system
device suspend phase and device noirq suspend phase, the keyboard
ISR will be called and both keyboard depress and release interrupts
will be disabled, then keyboard will no longer be able to wake up
system. Another scenario would be, if a keyboard is kept depressed,
and then system goes into suspend, the expected behavior would be
when keyboard is released, system will be waked up, but current
implementation can NOT achieve that, because both depress and release
interrupts are disabled in ISR, and the event check is still in
progress.
To fix these issues, need to make sure keyboard's depress or release
interrupt is enabled after noirq device suspend phase, this patch
moves the suspend/resume callback to noirq suspend/resume phase, and
enable the corresponding interrupt according to current keyboard status.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/input/keyboard/imx_keypad.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index cf08f4a..97500a2 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -524,11 +524,12 @@ static int imx_keypad_probe(struct platform_device *pdev)
return 0;
}
-static int __maybe_unused imx_kbd_suspend(struct device *dev)
+static int __maybe_unused imx_kbd_noirq_suspend(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
struct imx_keypad *kbd = platform_get_drvdata(pdev);
struct input_dev *input_dev = kbd->input_dev;
+ unsigned short reg_val = readw(kbd->mmio_base + KPSR);
/* imx kbd can wake up system even clock is disabled */
mutex_lock(&input_dev->mutex);
@@ -538,13 +539,20 @@ static int __maybe_unused imx_kbd_suspend(struct device *dev)
mutex_unlock(&input_dev->mutex);
- if (device_may_wakeup(&pdev->dev))
+ if (device_may_wakeup(&pdev->dev)) {
+ if (reg_val & KBD_STAT_KPKD)
+ reg_val |= KBD_STAT_KRIE;
+ if (reg_val & KBD_STAT_KPKR)
+ reg_val |= KBD_STAT_KDIE;
+ writew(reg_val, kbd->mmio_base + KPSR);
+
enable_irq_wake(kbd->irq);
+ }
return 0;
}
-static int __maybe_unused imx_kbd_resume(struct device *dev)
+static int __maybe_unused imx_kbd_noirq_resume(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
struct imx_keypad *kbd = platform_get_drvdata(pdev);
@@ -568,7 +576,9 @@ static int __maybe_unused imx_kbd_resume(struct device *dev)
return ret;
}
-static SIMPLE_DEV_PM_OPS(imx_kbd_pm_ops, imx_kbd_suspend, imx_kbd_resume);
+static const struct dev_pm_ops imx_kbd_pm_ops = {
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_kbd_noirq_suspend, imx_kbd_noirq_resume)
+};
static struct platform_driver imx_keypad_driver = {
.driver = {
--
2.7.4
^ permalink raw reply related
* Re: [RESEND PATCH v6 07/11] power: supply: max77650: add support for battery charger
From: Linus Walleij @ 2019-04-04 3:45 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Dmitry Torokhov, Jacek Anaszewski,
Pavel Machek, Lee Jones, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Input, Linux LED Subsystem, Linux PM list,
Bartosz Golaszewski
In-Reply-To: <20190318174228.18194-8-brgl@bgdev.pl>
On Tue, Mar 19, 2019 at 12:42 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add basic support for the battery charger for max77650 PMIC.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
This looks like a clean and good driver to me.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v8 05/11] mfd: core: document mfd_add_devices()
From: Lee Jones @ 2019-04-04 7:04 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190403090108.24788-6-brgl@bgdev.pl>
On Wed, 03 Apr 2019, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add a kernel doc for mfd_add_devices().
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
> drivers/mfd/mfd-core.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
For my own reference:
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v8 06/11] mfd: max77650: new core mfd driver
From: Lee Jones @ 2019-04-04 7:15 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190403090108.24788-7-brgl@bgdev.pl>
On Wed, 03 Apr 2019, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add the core mfd driver for max77650 PMIC. We define five sub-devices
> for which the drivers will be added in subsequent patches.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/mfd/Kconfig | 14 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/max77650.c | 234 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/max77650.h | 59 +++++++++
> 4 files changed, 308 insertions(+)
> create mode 100644 drivers/mfd/max77650.c
> create mode 100644 include/linux/mfd/max77650.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 0ce2d8dfc5f1..ade04e124aa0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -733,6 +733,20 @@ config MFD_MAX77620
> provides common support for accessing the device; additional drivers
> must be enabled in order to use the functionality of the device.
>
> +config MFD_MAX77650
> + tristate "Maxim MAX77650/77651 PMIC Support"
> + depends on I2C
> + depends on OF || COMPILE_TEST
> + select MFD_CORE
> + select REGMAP_I2C
> + help
> + Say Y here to add support for Maxim Semiconductor MAX77650 and
> + MAX77651 Power Management ICs. This is the core multifunction
> + driver for interacting with the device. The module name is
> + 'max77650'. Additional drivers can be enabled in order to use
> + the following functionalities of the device: GPIO, regulator,
> + charger, LED, onkey.
> +
> config MFD_MAX77686
> tristate "Maxim Semiconductor MAX77686/802 PMIC Support"
> depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index b4569ed7f3f3..5727d099c16f 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -155,6 +155,7 @@ obj-$(CONFIG_MFD_DA9150) += da9150-core.o
>
> obj-$(CONFIG_MFD_MAX14577) += max14577.o
> obj-$(CONFIG_MFD_MAX77620) += max77620.o
> +obj-$(CONFIG_MFD_MAX77650) += max77650.o
> obj-$(CONFIG_MFD_MAX77686) += max77686.o
> obj-$(CONFIG_MFD_MAX77693) += max77693.o
> obj-$(CONFIG_MFD_MAX77843) += max77843.o
> diff --git a/drivers/mfd/max77650.c b/drivers/mfd/max77650.c
> new file mode 100644
> index 000000000000..7a6c0a5cf602
> --- /dev/null
> +++ b/drivers/mfd/max77650.c
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (C) 2018 BayLibre SAS
> +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> +//
> +// Core MFD driver for MAXIM 77650/77651 charger/power-supply.
> +// Programming manual: https://pdfserv.maximintegrated.com/en/an/AN6428.pdf
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max77650.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +#define MAX77650_INT_GPI_F_MSK BIT(0)
> +#define MAX77650_INT_GPI_R_MSK BIT(1)
> +#define MAX77650_INT_GPI_MSK \
> + (MAX77650_INT_GPI_F_MSK | MAX77650_INT_GPI_R_MSK)
> +#define MAX77650_INT_nEN_F_MSK BIT(2)
> +#define MAX77650_INT_nEN_R_MSK BIT(3)
> +#define MAX77650_INT_TJAL1_R_MSK BIT(4)
> +#define MAX77650_INT_TJAL2_R_MSK BIT(5)
> +#define MAX77650_INT_DOD_R_MSK BIT(6)
> +
> +#define MAX77650_INT_THM_MSK BIT(0)
> +#define MAX77650_INT_CHG_MSK BIT(1)
> +#define MAX77650_INT_CHGIN_MSK BIT(2)
> +#define MAX77650_INT_TJ_REG_MSK BIT(3)
> +#define MAX77650_INT_CHGIN_CTRL_MSK BIT(4)
> +#define MAX77650_INT_SYS_CTRL_MSK BIT(5)
> +#define MAX77650_INT_SYS_CNFG_MSK BIT(6)
> +
> +#define MAX77650_INT_GLBL_OFFSET 0
> +#define MAX77650_INT_CHG_OFFSET 1
> +
> +#define MAX77650_SBIA_LPM_MASK BIT(5)
> +#define MAX77650_SBIA_LPM_DISABLED 0x00
> +
> +enum {
> + MAX77650_INT_GPI,
> + MAX77650_INT_nEN_F,
> + MAX77650_INT_nEN_R,
> + MAX77650_INT_TJAL1_R,
> + MAX77650_INT_TJAL2_R,
> + MAX77650_INT_DOD_R,
> + MAX77650_INT_THM,
> + MAX77650_INT_CHG,
> + MAX77650_INT_CHGIN,
> + MAX77650_INT_TJ_REG,
> + MAX77650_INT_CHGIN_CTRL,
> + MAX77650_INT_SYS_CTRL,
> + MAX77650_INT_SYS_CNFG,
> +};
> +
> +static const struct resource max77650_charger_resources[] = {
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_CHG, "CHG"),
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_CHGIN, "CHGIN"),
> +};
> +
> +static const struct resource max77650_gpio_resources[] = {
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_GPI, "GPI"),
> +};
> +
> +static const struct resource max77650_onkey_resources[] = {
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_nEN_F, "nEN_F"),
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_nEN_R, "nEN_R"),
> +};
> +
> +static const struct mfd_cell max77650_cells[] = {
> + {
> + .name = "max77650-regulator",
> + .of_compatible = "maxim,max77650-regulator",
> + },
> + {
Nit: Place these all on the same line, like:
}, {
> + .name = "max77650-charger",
> + .of_compatible = "maxim,max77650-charger",
> + .resources = max77650_charger_resources,
> + .num_resources = ARRAY_SIZE(max77650_charger_resources),
> + },
> + {
> + .name = "max77650-gpio",
> + .of_compatible = "maxim,max77650-gpio",
> + .resources = max77650_gpio_resources,
> + .num_resources = ARRAY_SIZE(max77650_gpio_resources),
> + },
> + {
> + .name = "max77650-led",
> + .of_compatible = "maxim,max77650-led",
> + },
> + {
> + .name = "max77650-onkey",
> + .of_compatible = "maxim,max77650-onkey",
> + .resources = max77650_onkey_resources,
> + .num_resources = ARRAY_SIZE(max77650_onkey_resources),
> + },
> +};
> +
> +static const struct regmap_irq max77650_irqs[] = {
> + [MAX77650_INT_GPI] = {
> + .reg_offset = MAX77650_INT_GLBL_OFFSET,
> + .mask = MAX77650_INT_GPI_MSK,
> + .type = {
> + .type_falling_val = MAX77650_INT_GPI_F_MSK,
> + .type_rising_val = MAX77650_INT_GPI_R_MSK,
> + .types_supported = IRQ_TYPE_EDGE_BOTH,
Not really a fan of these silly double/triple tabs everywhere. I like
a straight line as much as anyone, but I'd probably remove them from
here.
> + },
> + },
> + REGMAP_IRQ_REG(MAX77650_INT_nEN_F,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_nEN_F_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_nEN_R,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_nEN_R_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_TJAL1_R,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_TJAL1_R_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_TJAL2_R,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_TJAL2_R_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_DOD_R,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_DOD_R_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_THM,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_THM_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_CHG,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_CHG_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_CHGIN,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_CHGIN_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_TJ_REG,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_TJ_REG_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_CHGIN_CTRL,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_CHGIN_CTRL_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_SYS_CTRL,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_SYS_CTRL_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_SYS_CNFG,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_SYS_CNFG_MSK),
> +};
> +
> +static const struct regmap_irq_chip max77650_irq_chip = {
> + .name = "max77650-irq",
> + .irqs = max77650_irqs,
> + .num_irqs = ARRAY_SIZE(max77650_irqs),
> + .num_regs = 2,
> + .status_base = MAX77650_REG_INT_GLBL,
> + .mask_base = MAX77650_REG_INTM_GLBL,
> + .type_in_mask = true,
> + .type_invert = true,
> + .init_ack_masked = true,
> + .clear_on_unmask = true,
> +};
> +
> +static const struct regmap_config max77650_regmap_config = {
> + .name = "max77650",
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static int max77650_i2c_probe(struct i2c_client *i2c)
> +{
> + struct regmap_irq_chip_data *irq_data;
> + struct device *dev = &i2c->dev;
> + struct irq_domain *domain;
> + struct regmap *map;
> + unsigned int val;
> + int rv;
> +
> + map = devm_regmap_init_i2c(i2c, &max77650_regmap_config);
> + if (IS_ERR(map)) {
> + dev_err(dev, "unable to initialize i2c regmap\n");
"Unable to initialise I2C Regmap"
> + return PTR_ERR(map);
> + }
> +
> + rv = regmap_read(map, MAX77650_REG_CID, &val);
> + if (rv) {
> + dev_err(dev, "unable to read the CID from device\n");
"Unable to read Chip ID"
> + return rv;
> + }
> +
> + switch (MAX77650_CID_BITS(val)) {
> + case MAX77650_CID_77650A:
> + case MAX77650_CID_77650C:
> + case MAX77650_CID_77651A:
> + case MAX77650_CID_77651B:
> + break;
> + default:
So you're exiting silently here. What is the user going to think?
"Chip not supported - ID: %d"
> + return -ENODEV;
> + }
> +
> + /*
> + * This IC has a low-power mode which reduces the quiescent current
> + * consumption to ~5.6uA but is only suitable for systems consuming
> + * less than ~2mA. Since this is not likely the case even on
> + * linux-based wearables - keep the chip in normal power mode.
> + */
> + rv = regmap_update_bits(map,
> + MAX77650_REG_CNFG_GLBL,
> + MAX77650_SBIA_LPM_MASK,
> + MAX77650_SBIA_LPM_DISABLED);
> + if (rv) {
> + dev_err(dev, "unable to change the power mode\n");
"Unable ..."
> + return rv;
> + }
> +
> + rv = devm_regmap_add_irq_chip(dev, map, i2c->irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &max77650_irq_chip, &irq_data);
> + if (rv) {
> + dev_err(dev, "unable to add the regmap irq chip\n");
"Unable to add Regmap IRQ chip"
> + return rv;
> + }
> +
> + domain = regmap_irq_get_domain(irq_data);
No error checking?
> + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> + max77650_cells, ARRAY_SIZE(max77650_cells),
> + NULL, 0, domain);
> +}
> +
> +static const struct of_device_id max77650_of_match[] = {
> + { .compatible = "maxim,max77650" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max77650_of_match);
> +
> +static struct i2c_driver max77650_i2c_driver = {
> + .driver = {
> + .name = "max77650",
> + .of_match_table = of_match_ptr(max77650_of_match),
> + },
> + .probe_new = max77650_i2c_probe,
I wrote this years ago. Hasn't it gone away yet?
> +};
> +module_i2c_driver(max77650_i2c_driver);
> +
> +MODULE_DESCRIPTION("MAXIM 77650/77651 multi-function core driver");
> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/max77650.h b/include/linux/mfd/max77650.h
> new file mode 100644
> index 000000000000..c809e211a8cd
> --- /dev/null
> +++ b/include/linux/mfd/max77650.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 BayLibre SAS
> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> + *
> + * Common definitions for MAXIM 77650/77651 charger/power-supply.
> + */
> +
> +#ifndef MAX77650_H
> +#define MAX77650_H
> +
> +#include <linux/bits.h>
> +
> +#define MAX77650_REG_INT_GLBL 0x00
> +#define MAX77650_REG_INT_CHG 0x01
> +#define MAX77650_REG_STAT_CHG_A 0x02
> +#define MAX77650_REG_STAT_CHG_B 0x03
> +#define MAX77650_REG_ERCFLAG 0x04
> +#define MAX77650_REG_STAT_GLBL 0x05
> +#define MAX77650_REG_INTM_GLBL 0x06
> +#define MAX77650_REG_INTM_CHG 0x07
> +#define MAX77650_REG_CNFG_GLBL 0x10
> +#define MAX77650_REG_CID 0x11
> +#define MAX77650_REG_CNFG_GPIO 0x12
> +#define MAX77650_REG_CNFG_CHG_A 0x18
> +#define MAX77650_REG_CNFG_CHG_B 0x19
> +#define MAX77650_REG_CNFG_CHG_C 0x1a
> +#define MAX77650_REG_CNFG_CHG_D 0x1b
> +#define MAX77650_REG_CNFG_CHG_E 0x1c
> +#define MAX77650_REG_CNFG_CHG_F 0x1d
> +#define MAX77650_REG_CNFG_CHG_G 0x1e
> +#define MAX77650_REG_CNFG_CHG_H 0x1f
> +#define MAX77650_REG_CNFG_CHG_I 0x20
> +#define MAX77650_REG_CNFG_SBB_TOP 0x28
> +#define MAX77650_REG_CNFG_SBB0_A 0x29
> +#define MAX77650_REG_CNFG_SBB0_B 0x2a
> +#define MAX77650_REG_CNFG_SBB1_A 0x2b
> +#define MAX77650_REG_CNFG_SBB1_B 0x2c
> +#define MAX77650_REG_CNFG_SBB2_A 0x2d
> +#define MAX77650_REG_CNFG_SBB2_B 0x2e
> +#define MAX77650_REG_CNFG_LDO_A 0x38
> +#define MAX77650_REG_CNFG_LDO_B 0x39
> +#define MAX77650_REG_CNFG_LED0_A 0x40
> +#define MAX77650_REG_CNFG_LED1_A 0x41
> +#define MAX77650_REG_CNFG_LED2_A 0x42
> +#define MAX77650_REG_CNFG_LED0_B 0x43
> +#define MAX77650_REG_CNFG_LED1_B 0x44
> +#define MAX77650_REG_CNFG_LED2_B 0x45
> +#define MAX77650_REG_CNFG_LED_TOP 0x46
> +
> +#define MAX77650_CID_MASK GENMASK(3, 0)
> +#define MAX77650_CID_BITS(_reg) (_reg & MAX77650_CID_MASK)
> +
> +#define MAX77650_CID_77650A 0x03
> +#define MAX77650_CID_77650C 0x0a
> +#define MAX77650_CID_77651A 0x06
> +#define MAX77650_CID_77651B 0x08
> +
> +#endif /* MAX77650_H */
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 0/5] Input: Use devm_platform_ioremap_resource()
From: Mukesh Ojha @ 2019-04-04 7:17 UTC (permalink / raw)
To: dmitry.torokhov, shawnguo, s.hauer, linux-imx, linux-input,
linux-arm-kernel, linux-kernel
Cc: Mukesh Ojha
Hi All,
This is just a minor change to promote using of this api
devm_platform_ioremap_resource() and nothing else functional
change it does.
Also, there are patches where only ioremap is used without requesting
the mem region so this devm_platform_ioremap_resource() take cares all
of that in single call.
Mukesh Ojha (5):
Input: fsl-imx25-tcq: Use devm_platform_ioremap_resource()
Input: mxs-lradc-ts.c: Use devm_platform_ioremap_resource()
Input: s3c2410_ts: Use devm_platform_ioremap_resource()
Input: sun4i-ts: Use devm_platform_ioremap_resource()
Input: ts4800-ts: Use devm_platform_ioremap_resource()
drivers/input/touchscreen/fsl-imx25-tcq.c | 4 +---
drivers/input/touchscreen/mxs-lradc-ts.c | 6 +-----
drivers/input/touchscreen/s3c2410_ts.c | 10 +---------
drivers/input/touchscreen/sun4i-ts.c | 3 +--
drivers/input/touchscreen/ts4800-ts.c | 4 +---
5 files changed, 5 insertions(+), 22 deletions(-)
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH 1/5] Input: fsl-imx25-tcq: Use devm_platform_ioremap_resource()
From: Mukesh Ojha @ 2019-04-04 7:17 UTC (permalink / raw)
To: dmitry.torokhov, shawnguo, s.hauer, linux-imx, linux-input,
linux-arm-kernel, linux-kernel
Cc: Mukesh Ojha
In-Reply-To: <1554362243-2888-1-git-send-email-mojha@codeaurora.org>
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <mojha@codeaurora.org>
---
drivers/input/touchscreen/fsl-imx25-tcq.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/fsl-imx25-tcq.c b/drivers/input/touchscreen/fsl-imx25-tcq.c
index 1d6c8f4..b66df8a 100644
--- a/drivers/input/touchscreen/fsl-imx25-tcq.c
+++ b/drivers/input/touchscreen/fsl-imx25-tcq.c
@@ -503,7 +503,6 @@ static int mx25_tcq_probe(struct platform_device *pdev)
struct input_dev *idev;
struct mx25_tcq_priv *priv;
struct mx25_tsadc *tsadc = dev_get_drvdata(dev->parent);
- struct resource *res;
void __iomem *mem;
int error;
@@ -512,8 +511,7 @@ static int mx25_tcq_probe(struct platform_device *pdev)
return -ENOMEM;
priv->dev = dev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- mem = devm_ioremap_resource(dev, res);
+ mem = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(mem))
return PTR_ERR(mem);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 2/5] Input: mxs-lradc-ts.c: Use devm_platform_ioremap_resource()
From: Mukesh Ojha @ 2019-04-04 7:17 UTC (permalink / raw)
To: dmitry.torokhov, shawnguo, s.hauer, linux-imx, linux-input,
linux-arm-kernel, linux-kernel
Cc: Mukesh Ojha
In-Reply-To: <1554362243-2888-1-git-send-email-mojha@codeaurora.org>
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <mojha@codeaurora.org>
---
drivers/input/touchscreen/mxs-lradc-ts.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/mxs-lradc-ts.c b/drivers/input/touchscreen/mxs-lradc-ts.c
index c850b51..af047fa 100644
--- a/drivers/input/touchscreen/mxs-lradc-ts.c
+++ b/drivers/input/touchscreen/mxs-lradc-ts.c
@@ -615,7 +615,6 @@ static int mxs_lradc_ts_probe(struct platform_device *pdev)
struct device_node *node = dev->parent->of_node;
struct mxs_lradc *lradc = dev_get_drvdata(dev->parent);
struct mxs_lradc_ts *ts;
- struct resource *iores;
int ret, irq, virq, i;
u32 ts_wires = 0, adapt;
@@ -629,10 +628,7 @@ static int mxs_lradc_ts_probe(struct platform_device *pdev)
ts->dev = dev;
spin_lock_init(&ts->lock);
- iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!iores)
- return -EINVAL;
- ts->base = devm_ioremap(dev, iores->start, resource_size(iores));
+ ts->base = devm_platform_ioremap_resource(pdev, 0);
if (!ts->base)
return -ENOMEM;
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 3/5] Input: s3c2410_ts: Use devm_platform_ioremap_resource()
From: Mukesh Ojha @ 2019-04-04 7:17 UTC (permalink / raw)
To: dmitry.torokhov, shawnguo, s.hauer, linux-imx, linux-input,
linux-arm-kernel, linux-kernel
Cc: Mukesh Ojha
In-Reply-To: <1554362243-2888-1-git-send-email-mojha@codeaurora.org>
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <mojha@codeaurora.org>
---
drivers/input/touchscreen/s3c2410_ts.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/input/touchscreen/s3c2410_ts.c b/drivers/input/touchscreen/s3c2410_ts.c
index 1173890..e11cdae 100644
--- a/drivers/input/touchscreen/s3c2410_ts.c
+++ b/drivers/input/touchscreen/s3c2410_ts.c
@@ -242,7 +242,6 @@ static int s3c2410ts_probe(struct platform_device *pdev)
struct s3c2410_ts_mach_info *info;
struct device *dev = &pdev->dev;
struct input_dev *input_dev;
- struct resource *res;
int ret = -EINVAL;
/* Initialise input stuff */
@@ -277,14 +276,7 @@ static int s3c2410ts_probe(struct platform_device *pdev)
goto err_clk;
}
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(dev, "no resource for registers\n");
- ret = -ENOENT;
- goto err_clk;
- }
-
- ts.io = ioremap(res->start, resource_size(res));
+ ts.io = devm_platform_ioremap_resource(pdev, 0);
if (ts.io == NULL) {
dev_err(dev, "cannot map registers\n");
ret = -ENOMEM;
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 4/5] Input: sun4i-ts: Use devm_platform_ioremap_resource()
From: Mukesh Ojha @ 2019-04-04 7:17 UTC (permalink / raw)
To: dmitry.torokhov, shawnguo, s.hauer, linux-imx, linux-input,
linux-arm-kernel, linux-kernel
Cc: Mukesh Ojha
In-Reply-To: <1554362243-2888-1-git-send-email-mojha@codeaurora.org>
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <mojha@codeaurora.org>
---
drivers/input/touchscreen/sun4i-ts.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
index d2e14d9..bbb0104 100644
--- a/drivers/input/touchscreen/sun4i-ts.c
+++ b/drivers/input/touchscreen/sun4i-ts.c
@@ -309,8 +309,7 @@ static int sun4i_ts_probe(struct platform_device *pdev)
input_set_drvdata(ts->input, ts);
}
- ts->base = devm_ioremap_resource(dev,
- platform_get_resource(pdev, IORESOURCE_MEM, 0));
+ ts->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(ts->base))
return PTR_ERR(ts->base);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 5/5] Input: ts4800-ts: Use devm_platform_ioremap_resource()
From: Mukesh Ojha @ 2019-04-04 7:17 UTC (permalink / raw)
To: dmitry.torokhov, shawnguo, s.hauer, linux-imx, linux-input,
linux-arm-kernel, linux-kernel
Cc: Mukesh Ojha
In-Reply-To: <1554362243-2888-1-git-send-email-mojha@codeaurora.org>
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <mojha@codeaurora.org>
---
drivers/input/touchscreen/ts4800-ts.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/ts4800-ts.c b/drivers/input/touchscreen/ts4800-ts.c
index fed73ee..5b4f536 100644
--- a/drivers/input/touchscreen/ts4800-ts.c
+++ b/drivers/input/touchscreen/ts4800-ts.c
@@ -148,7 +148,6 @@ static int ts4800_ts_probe(struct platform_device *pdev)
{
struct input_polled_dev *poll_dev;
struct ts4800_ts *ts;
- struct resource *res;
int error;
ts = devm_kzalloc(&pdev->dev, sizeof(*ts), GFP_KERNEL);
@@ -159,8 +158,7 @@ static int ts4800_ts_probe(struct platform_device *pdev)
if (error)
return error;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- ts->base = devm_ioremap_resource(&pdev->dev, res);
+ ts->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(ts->base))
return PTR_ERR(ts->base);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: [PATCH v8 06/11] mfd: max77650: new core mfd driver
From: Bartosz Golaszewski @ 2019-04-04 7:30 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, Linux Kernel Mailing List,
open list:GPIO SUBSYSTEM, devicetree, Linux Input,
Linux LED Subsystem, Linux PM list, Bartosz Golaszewski
In-Reply-To: <20190404071538.GH6830@dell>
czw., 4 kwi 2019 o 09:15 Lee Jones <lee.jones@linaro.org> napisał(a):
>
> On Wed, 03 Apr 2019, Bartosz Golaszewski wrote:
>
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Add the core mfd driver for max77650 PMIC. We define five sub-devices
> > for which the drivers will be added in subsequent patches.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> > drivers/mfd/Kconfig | 14 +++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/max77650.c | 234 +++++++++++++++++++++++++++++++++++
> > include/linux/mfd/max77650.h | 59 +++++++++
> > 4 files changed, 308 insertions(+)
> > create mode 100644 drivers/mfd/max77650.c
> > create mode 100644 include/linux/mfd/max77650.h
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 0ce2d8dfc5f1..ade04e124aa0 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -733,6 +733,20 @@ config MFD_MAX77620
> > provides common support for accessing the device; additional drivers
> > must be enabled in order to use the functionality of the device.
> >
> > +config MFD_MAX77650
> > + tristate "Maxim MAX77650/77651 PMIC Support"
> > + depends on I2C
> > + depends on OF || COMPILE_TEST
> > + select MFD_CORE
> > + select REGMAP_I2C
> > + help
> > + Say Y here to add support for Maxim Semiconductor MAX77650 and
> > + MAX77651 Power Management ICs. This is the core multifunction
> > + driver for interacting with the device. The module name is
> > + 'max77650'. Additional drivers can be enabled in order to use
> > + the following functionalities of the device: GPIO, regulator,
> > + charger, LED, onkey.
> > +
> > config MFD_MAX77686
> > tristate "Maxim Semiconductor MAX77686/802 PMIC Support"
> > depends on I2C
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index b4569ed7f3f3..5727d099c16f 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -155,6 +155,7 @@ obj-$(CONFIG_MFD_DA9150) += da9150-core.o
> >
> > obj-$(CONFIG_MFD_MAX14577) += max14577.o
> > obj-$(CONFIG_MFD_MAX77620) += max77620.o
> > +obj-$(CONFIG_MFD_MAX77650) += max77650.o
> > obj-$(CONFIG_MFD_MAX77686) += max77686.o
> > obj-$(CONFIG_MFD_MAX77693) += max77693.o
> > obj-$(CONFIG_MFD_MAX77843) += max77843.o
> > diff --git a/drivers/mfd/max77650.c b/drivers/mfd/max77650.c
> > new file mode 100644
> > index 000000000000..7a6c0a5cf602
> > --- /dev/null
> > +++ b/drivers/mfd/max77650.c
> > @@ -0,0 +1,234 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Copyright (C) 2018 BayLibre SAS
> > +// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > +//
> > +// Core MFD driver for MAXIM 77650/77651 charger/power-supply.
> > +// Programming manual: https://pdfserv.maximintegrated.com/en/an/AN6428.pdf
> > +
> > +#include <linux/i2c.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irq.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/mfd/max77650.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/regmap.h>
> > +
> > +#define MAX77650_INT_GPI_F_MSK BIT(0)
> > +#define MAX77650_INT_GPI_R_MSK BIT(1)
> > +#define MAX77650_INT_GPI_MSK \
> > + (MAX77650_INT_GPI_F_MSK | MAX77650_INT_GPI_R_MSK)
> > +#define MAX77650_INT_nEN_F_MSK BIT(2)
> > +#define MAX77650_INT_nEN_R_MSK BIT(3)
> > +#define MAX77650_INT_TJAL1_R_MSK BIT(4)
> > +#define MAX77650_INT_TJAL2_R_MSK BIT(5)
> > +#define MAX77650_INT_DOD_R_MSK BIT(6)
> > +
> > +#define MAX77650_INT_THM_MSK BIT(0)
> > +#define MAX77650_INT_CHG_MSK BIT(1)
> > +#define MAX77650_INT_CHGIN_MSK BIT(2)
> > +#define MAX77650_INT_TJ_REG_MSK BIT(3)
> > +#define MAX77650_INT_CHGIN_CTRL_MSK BIT(4)
> > +#define MAX77650_INT_SYS_CTRL_MSK BIT(5)
> > +#define MAX77650_INT_SYS_CNFG_MSK BIT(6)
> > +
> > +#define MAX77650_INT_GLBL_OFFSET 0
> > +#define MAX77650_INT_CHG_OFFSET 1
> > +
> > +#define MAX77650_SBIA_LPM_MASK BIT(5)
> > +#define MAX77650_SBIA_LPM_DISABLED 0x00
> > +
> > +enum {
> > + MAX77650_INT_GPI,
> > + MAX77650_INT_nEN_F,
> > + MAX77650_INT_nEN_R,
> > + MAX77650_INT_TJAL1_R,
> > + MAX77650_INT_TJAL2_R,
> > + MAX77650_INT_DOD_R,
> > + MAX77650_INT_THM,
> > + MAX77650_INT_CHG,
> > + MAX77650_INT_CHGIN,
> > + MAX77650_INT_TJ_REG,
> > + MAX77650_INT_CHGIN_CTRL,
> > + MAX77650_INT_SYS_CTRL,
> > + MAX77650_INT_SYS_CNFG,
> > +};
> > +
> > +static const struct resource max77650_charger_resources[] = {
> > + DEFINE_RES_IRQ_NAMED(MAX77650_INT_CHG, "CHG"),
> > + DEFINE_RES_IRQ_NAMED(MAX77650_INT_CHGIN, "CHGIN"),
> > +};
> > +
> > +static const struct resource max77650_gpio_resources[] = {
> > + DEFINE_RES_IRQ_NAMED(MAX77650_INT_GPI, "GPI"),
> > +};
> > +
> > +static const struct resource max77650_onkey_resources[] = {
> > + DEFINE_RES_IRQ_NAMED(MAX77650_INT_nEN_F, "nEN_F"),
> > + DEFINE_RES_IRQ_NAMED(MAX77650_INT_nEN_R, "nEN_R"),
> > +};
> > +
> > +static const struct mfd_cell max77650_cells[] = {
> > + {
> > + .name = "max77650-regulator",
> > + .of_compatible = "maxim,max77650-regulator",
>
> > + },
> > + {
>
> Nit: Place these all on the same line, like:
>
> }, {
>
> > + .name = "max77650-charger",
> > + .of_compatible = "maxim,max77650-charger",
> > + .resources = max77650_charger_resources,
> > + .num_resources = ARRAY_SIZE(max77650_charger_resources),
> > + },
> > + {
> > + .name = "max77650-gpio",
> > + .of_compatible = "maxim,max77650-gpio",
> > + .resources = max77650_gpio_resources,
> > + .num_resources = ARRAY_SIZE(max77650_gpio_resources),
> > + },
> > + {
> > + .name = "max77650-led",
> > + .of_compatible = "maxim,max77650-led",
> > + },
> > + {
> > + .name = "max77650-onkey",
> > + .of_compatible = "maxim,max77650-onkey",
> > + .resources = max77650_onkey_resources,
> > + .num_resources = ARRAY_SIZE(max77650_onkey_resources),
> > + },
> > +};
> > +
> > +static const struct regmap_irq max77650_irqs[] = {
> > + [MAX77650_INT_GPI] = {
> > + .reg_offset = MAX77650_INT_GLBL_OFFSET,
> > + .mask = MAX77650_INT_GPI_MSK,
> > + .type = {
> > + .type_falling_val = MAX77650_INT_GPI_F_MSK,
> > + .type_rising_val = MAX77650_INT_GPI_R_MSK,
> > + .types_supported = IRQ_TYPE_EDGE_BOTH,
>
> Not really a fan of these silly double/triple tabs everywhere. I like
> a straight line as much as anyone, but I'd probably remove them from
> here.
>
> > + },
> > + },
> > + REGMAP_IRQ_REG(MAX77650_INT_nEN_F,
> > + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_nEN_F_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_nEN_R,
> > + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_nEN_R_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_TJAL1_R,
> > + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_TJAL1_R_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_TJAL2_R,
> > + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_TJAL2_R_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_DOD_R,
> > + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_DOD_R_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_THM,
> > + MAX77650_INT_CHG_OFFSET, MAX77650_INT_THM_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_CHG,
> > + MAX77650_INT_CHG_OFFSET, MAX77650_INT_CHG_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_CHGIN,
> > + MAX77650_INT_CHG_OFFSET, MAX77650_INT_CHGIN_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_TJ_REG,
> > + MAX77650_INT_CHG_OFFSET, MAX77650_INT_TJ_REG_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_CHGIN_CTRL,
> > + MAX77650_INT_CHG_OFFSET, MAX77650_INT_CHGIN_CTRL_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_SYS_CTRL,
> > + MAX77650_INT_CHG_OFFSET, MAX77650_INT_SYS_CTRL_MSK),
> > + REGMAP_IRQ_REG(MAX77650_INT_SYS_CNFG,
> > + MAX77650_INT_CHG_OFFSET, MAX77650_INT_SYS_CNFG_MSK),
> > +};
> > +
> > +static const struct regmap_irq_chip max77650_irq_chip = {
> > + .name = "max77650-irq",
> > + .irqs = max77650_irqs,
> > + .num_irqs = ARRAY_SIZE(max77650_irqs),
> > + .num_regs = 2,
> > + .status_base = MAX77650_REG_INT_GLBL,
> > + .mask_base = MAX77650_REG_INTM_GLBL,
> > + .type_in_mask = true,
> > + .type_invert = true,
> > + .init_ack_masked = true,
> > + .clear_on_unmask = true,
> > +};
> > +
> > +static const struct regmap_config max77650_regmap_config = {
> > + .name = "max77650",
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > +};
> > +
> > +static int max77650_i2c_probe(struct i2c_client *i2c)
> > +{
> > + struct regmap_irq_chip_data *irq_data;
> > + struct device *dev = &i2c->dev;
> > + struct irq_domain *domain;
> > + struct regmap *map;
> > + unsigned int val;
> > + int rv;
> > +
> > + map = devm_regmap_init_i2c(i2c, &max77650_regmap_config);
> > + if (IS_ERR(map)) {
> > + dev_err(dev, "unable to initialize i2c regmap\n");
>
> "Unable to initialise I2C Regmap"
>
> > + return PTR_ERR(map);
> > + }
> > +
> > + rv = regmap_read(map, MAX77650_REG_CID, &val);
> > + if (rv) {
> > + dev_err(dev, "unable to read the CID from device\n");
>
> "Unable to read Chip ID"
>
> > + return rv;
> > + }
> > +
> > + switch (MAX77650_CID_BITS(val)) {
> > + case MAX77650_CID_77650A:
> > + case MAX77650_CID_77650C:
> > + case MAX77650_CID_77651A:
> > + case MAX77650_CID_77651B:
> > + break;
> > + default:
>
> So you're exiting silently here. What is the user going to think?
>
> "Chip not supported - ID: %d"
>
> > + return -ENODEV;
> > + }
> > +
> > + /*
> > + * This IC has a low-power mode which reduces the quiescent current
> > + * consumption to ~5.6uA but is only suitable for systems consuming
> > + * less than ~2mA. Since this is not likely the case even on
> > + * linux-based wearables - keep the chip in normal power mode.
> > + */
> > + rv = regmap_update_bits(map,
> > + MAX77650_REG_CNFG_GLBL,
> > + MAX77650_SBIA_LPM_MASK,
> > + MAX77650_SBIA_LPM_DISABLED);
> > + if (rv) {
> > + dev_err(dev, "unable to change the power mode\n");
>
> "Unable ..."
>
> > + return rv;
> > + }
> > +
> > + rv = devm_regmap_add_irq_chip(dev, map, i2c->irq,
> > + IRQF_ONESHOT | IRQF_SHARED, 0,
> > + &max77650_irq_chip, &irq_data);
> > + if (rv) {
> > + dev_err(dev, "unable to add the regmap irq chip\n");
>
> "Unable to add Regmap IRQ chip"
>
> > + return rv;
> > + }
> > +
> > + domain = regmap_irq_get_domain(irq_data);
>
> No error checking?
>
This can only fail is irq_data is NULL and since the previous function
succeeded we know it isn't.
Bart
> > + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> > + max77650_cells, ARRAY_SIZE(max77650_cells),
> > + NULL, 0, domain);
> > +}
> > +
> > +static const struct of_device_id max77650_of_match[] = {
> > + { .compatible = "maxim,max77650" },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, max77650_of_match);
> > +
> > +static struct i2c_driver max77650_i2c_driver = {
> > + .driver = {
> > + .name = "max77650",
> > + .of_match_table = of_match_ptr(max77650_of_match),
> > + },
> > + .probe_new = max77650_i2c_probe,
>
> I wrote this years ago. Hasn't it gone away yet?
>
> > +};
> > +module_i2c_driver(max77650_i2c_driver);
> > +
> > +MODULE_DESCRIPTION("MAXIM 77650/77651 multi-function core driver");
> > +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/include/linux/mfd/max77650.h b/include/linux/mfd/max77650.h
> > new file mode 100644
> > index 000000000000..c809e211a8cd
> > --- /dev/null
> > +++ b/include/linux/mfd/max77650.h
> > @@ -0,0 +1,59 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2018 BayLibre SAS
> > + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > + *
> > + * Common definitions for MAXIM 77650/77651 charger/power-supply.
> > + */
> > +
> > +#ifndef MAX77650_H
> > +#define MAX77650_H
> > +
> > +#include <linux/bits.h>
> > +
> > +#define MAX77650_REG_INT_GLBL 0x00
> > +#define MAX77650_REG_INT_CHG 0x01
> > +#define MAX77650_REG_STAT_CHG_A 0x02
> > +#define MAX77650_REG_STAT_CHG_B 0x03
> > +#define MAX77650_REG_ERCFLAG 0x04
> > +#define MAX77650_REG_STAT_GLBL 0x05
> > +#define MAX77650_REG_INTM_GLBL 0x06
> > +#define MAX77650_REG_INTM_CHG 0x07
> > +#define MAX77650_REG_CNFG_GLBL 0x10
> > +#define MAX77650_REG_CID 0x11
> > +#define MAX77650_REG_CNFG_GPIO 0x12
> > +#define MAX77650_REG_CNFG_CHG_A 0x18
> > +#define MAX77650_REG_CNFG_CHG_B 0x19
> > +#define MAX77650_REG_CNFG_CHG_C 0x1a
> > +#define MAX77650_REG_CNFG_CHG_D 0x1b
> > +#define MAX77650_REG_CNFG_CHG_E 0x1c
> > +#define MAX77650_REG_CNFG_CHG_F 0x1d
> > +#define MAX77650_REG_CNFG_CHG_G 0x1e
> > +#define MAX77650_REG_CNFG_CHG_H 0x1f
> > +#define MAX77650_REG_CNFG_CHG_I 0x20
> > +#define MAX77650_REG_CNFG_SBB_TOP 0x28
> > +#define MAX77650_REG_CNFG_SBB0_A 0x29
> > +#define MAX77650_REG_CNFG_SBB0_B 0x2a
> > +#define MAX77650_REG_CNFG_SBB1_A 0x2b
> > +#define MAX77650_REG_CNFG_SBB1_B 0x2c
> > +#define MAX77650_REG_CNFG_SBB2_A 0x2d
> > +#define MAX77650_REG_CNFG_SBB2_B 0x2e
> > +#define MAX77650_REG_CNFG_LDO_A 0x38
> > +#define MAX77650_REG_CNFG_LDO_B 0x39
> > +#define MAX77650_REG_CNFG_LED0_A 0x40
> > +#define MAX77650_REG_CNFG_LED1_A 0x41
> > +#define MAX77650_REG_CNFG_LED2_A 0x42
> > +#define MAX77650_REG_CNFG_LED0_B 0x43
> > +#define MAX77650_REG_CNFG_LED1_B 0x44
> > +#define MAX77650_REG_CNFG_LED2_B 0x45
> > +#define MAX77650_REG_CNFG_LED_TOP 0x46
> > +
> > +#define MAX77650_CID_MASK GENMASK(3, 0)
> > +#define MAX77650_CID_BITS(_reg) (_reg & MAX77650_CID_MASK)
> > +
> > +#define MAX77650_CID_77650A 0x03
> > +#define MAX77650_CID_77650C 0x0a
> > +#define MAX77650_CID_77651A 0x06
> > +#define MAX77650_CID_77651B 0x08
> > +
> > +#endif /* MAX77650_H */
>
> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH][next] HID: intel-ish-hid: fix spelling mistake "multipe" -> "multiple"
From: Colin King @ 2019-04-04 7:58 UTC (permalink / raw)
To: Srinivas Pandruvada, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel
Cc: kernel-janitors
From: Colin Ian King <colin.king@canonical.com>
There is a spelling mistake in a dev_err message, fix it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
index e770e220bd73..22ba21457035 100644
--- a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
+++ b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
@@ -525,7 +525,7 @@ static int ish_query_loader_prop(struct ishtp_cl_data *client_data,
if ((fw_info->ldr_capability.xfer_mode & LOADER_XFER_MODE_DIRECT_DMA) &&
(fw_info->ldr_capability.max_dma_buf_size % L1_CACHE_BYTES)) {
dev_err(cl_data_to_dev(client_data),
- "Shim firmware loader buffer size %d should be multipe of cacheline\n",
+ "Shim firmware loader buffer size %d should be multiple of cacheline\n",
fw_info->ldr_capability.max_dma_buf_size);
return -EINVAL;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH][next] HID: intel-ish-hid: fix spelling mistake "multipe" -> "multiple"
From: Mukesh Ojha @ 2019-04-04 8:55 UTC (permalink / raw)
To: Colin King, Srinivas Pandruvada, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel
Cc: kernel-janitors
In-Reply-To: <20190404075857.12411-1-colin.king@canonical.com>
On 4/4/2019 1:28 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a spelling mistake in a dev_err message, fix it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
Cheers,
-Mukesh
> ---
> drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> index e770e220bd73..22ba21457035 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> @@ -525,7 +525,7 @@ static int ish_query_loader_prop(struct ishtp_cl_data *client_data,
> if ((fw_info->ldr_capability.xfer_mode & LOADER_XFER_MODE_DIRECT_DMA) &&
> (fw_info->ldr_capability.max_dma_buf_size % L1_CACHE_BYTES)) {
> dev_err(cl_data_to_dev(client_data),
> - "Shim firmware loader buffer size %d should be multipe of cacheline\n",
> + "Shim firmware loader buffer size %d should be multiple of cacheline\n",
> fw_info->ldr_capability.max_dma_buf_size);
> return -EINVAL;
> }
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox