* Re: [PATCH v2 0/3] gpio-keys: Add support for specifying wakeup event action
From: JeffyChen @ 2018-02-23 9:43 UTC (permalink / raw)
To: Heiko Stübner
Cc: Kate Stewart, Mark Rutland, Emil Renner Berthing, stephen lu,
Catalin Marinas, Will Deacon, Joseph Lo, Brian Norris,
linux-rockchip, Matthias Kaehlcke, linux-input, Arvind Yadav,
dtor, devicetree, Arnd Bergmann, dianders, Rob Herring,
briannorris, Thomas Gleixner, linux-arm-kernel,
Greg Kroah-Hartman, Dmitry Torokhov, linux-kernel,
Philippe Ombredanne, Enric Balletbo i Serra <enric.bal>
In-Reply-To: <1541427.8RRpDbxO4f@diego>
Hi Heiko,
Thanks for your reply :)
On 02/14/2018 09:39 PM, Heiko Stübner wrote:
> Hi Jeffy,
>
> Am Samstag, 10. Februar 2018, 12:09:04 CET schrieb Jeffy Chen:
>> On chromebook kevin, we are using gpio-keys for pen insert event. But
>> we only want it to wakeup the system when ejecting the pen.
>>
>> So we may need to change the interrupt trigger type during suspending.
>>
>> Changes in v2:
>> Specify wakeup event action instead of irq trigger type as Brian
>> suggested.
>>
>> Jeffy Chen (3):
>> Input: gpio-keys - add support for wakeup event action
>> Input: gpio-keys - allow setting wakeup event action in DT
>> arm64: dts: rockchip: Avoid wakeup when inserting the pen
>
> The recipient-selection for the mail thread seems to be a bit off and
> I only got the cover-letter and patch 3/3.
> Therefore I won't really see if and when the Input-changes get
> accepted. So please ping me once that has happened so I can pick up
> the rockchip dts change for it.
oops, sorry, i'll add you to the CC list in the next version :)
>
>
> Thanks
> Heiko
>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 01/10] pwm: extend PWM framework with PWM modes
From: kbuild test robot @ 2018-02-24 20:49 UTC (permalink / raw)
To: Claudiu Beznea
Cc: mark.rutland, milo.kim, linux-fbdev, sean, devicetree, airlied,
mturquette, joonas.lahtinen, kamil, dri-devel, linux-kernel,
thierry.reding, alexandre.belloni, pavel, lee.jones, linux-clk,
linux-leds, daniel.thompson, linux-samsung-soc, shc_work, corbet,
linux-doc, linux, krzk, kgene, linux-input, linux, linux-media,
linux-pwm, jdelvare, b.zolnierkie, intel-gfx, jani.nikula,
robh+dt, jacek.anaszewski, rodrigo.vivi
In-Reply-To: <1519300881-8136-2-git-send-email-claudiu.beznea@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 2343 bytes --]
Hi Claudiu,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on pwm/for-next]
[also build test WARNING on v4.16-rc2 next-20180223]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Claudiu-Beznea/extend-PWM-framework-to-support-PWM-modes/20180225-024011
base: https://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git for-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All warnings (new ones prefixed by >>):
>> drivers//pwm/pwm-sun4i.c:36:0: warning: "PWM_MODE" redefined
#define PWM_MODE BIT(7)
In file included from drivers//pwm/pwm-sun4i.c:19:0:
include/linux/pwm.h:40:0: note: this is the location of the previous definition
#define PWM_MODE(name) BIT(PWM_MODE_##name##_BIT)
vim +/PWM_MODE +36 drivers//pwm/pwm-sun4i.c
09853ce7 Alexandre Belloni 2014-12-17 29
09853ce7 Alexandre Belloni 2014-12-17 30 #define PWMCH_OFFSET 15
09853ce7 Alexandre Belloni 2014-12-17 31 #define PWM_PRESCAL_MASK GENMASK(3, 0)
09853ce7 Alexandre Belloni 2014-12-17 32 #define PWM_PRESCAL_OFF 0
09853ce7 Alexandre Belloni 2014-12-17 33 #define PWM_EN BIT(4)
09853ce7 Alexandre Belloni 2014-12-17 34 #define PWM_ACT_STATE BIT(5)
09853ce7 Alexandre Belloni 2014-12-17 35 #define PWM_CLK_GATING BIT(6)
09853ce7 Alexandre Belloni 2014-12-17 @36 #define PWM_MODE BIT(7)
09853ce7 Alexandre Belloni 2014-12-17 37 #define PWM_PULSE BIT(8)
09853ce7 Alexandre Belloni 2014-12-17 38 #define PWM_BYPASS BIT(9)
09853ce7 Alexandre Belloni 2014-12-17 39
:::::: The code at line 36 was first introduced by commit
:::::: 09853ce7bc1003a490c7ee74a5705d7a7cf16b7d pwm: Add Allwinner SoC support
:::::: TO: Alexandre Belloni <alexandre.belloni@free-electrons.com>
:::::: CC: Thierry Reding <thierry.reding@gmail.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52627 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] Input: gpio_keys: Add level trigger support for GPIO keys
From: Baolin Wang @ 2018-02-26 6:24 UTC (permalink / raw)
To: Rob Herring
Cc: Dmitry Torokhov, Mark Rutland, Greg KH, stephen lu, Arvind Yadav,
Joseph Lo, Kate Stewart, Philippe Ombredanne, Thomas Gleixner,
linux-input, DTML, LKML, Mark Brown, Linus Walleij
In-Reply-To: <CAMz4kuL-VZ8XY5-ZLWfXLVXWb4zmxD-aaLQ-ka9M8V83y1B7Ag@mail.gmail.com>
Hi Rob,
On 21 February 2018 at 19:35, Baolin Wang <baolin.wang@linaro.org> wrote:
> Hi Rob,
>
> On 20 February 2018 at 02:11, Rob Herring <robh@kernel.org> wrote:
>> On Sun, Feb 11, 2018 at 02:55:04PM +0800, Baolin Wang wrote:
>>> On some platforms (such as Spreadtrum platform), the GPIO keys can only
>>> be triggered by level type. So this patch introduces one property to
>>> indicate if the GPIO trigger type is level trigger or edge trigger.
>>
>> If the parent interrupt controller only supports a certain trigger, then
>> it should ignore setting the trigger type.
>
> We still need to set high level type trigger or low level type trigger
> if it only supports level trigger.
>
>>
>>>
>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>> ---
>>> Changes since v1:
>>> - Diable the GPIO irq until reversing the GPIO level type.
>>> ---
>>> .../devicetree/bindings/input/gpio-keys.txt | 2 ++
>>> drivers/input/keyboard/gpio_keys.c | 26 +++++++++++++++++++-
>>> include/linux/gpio_keys.h | 1 +
>>> 3 files changed, 28 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
>>> index a949404..e3104bd 100644
>>> --- a/Documentation/devicetree/bindings/input/gpio-keys.txt
>>> +++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
>>> @@ -29,6 +29,8 @@ Optional subnode-properties:
>>> - linux,can-disable: Boolean, indicates that button is connected
>>> to dedicated (not shared) interrupt which can be disabled to
>>> suppress events from the button.
>>> + - gpio-key,level-trigger: Boolean, indicates that button's interrupt
>>> + type is level trigger. Otherwise it is edge trigger as default.
>>
>> No. Just use 'interrupts' instead of 'gpios' and specify the trigger
>> type. Or put both if you need to read the state.
>
> Okay, so something as below to get the level type from the
> 'interrupts' property.
> if (fwnode_property_read_u32(child, "interrupts", &button->level_type))
> button->level_type = IRQ_TYPE_NONE;
After more thinking, if we use 'interrupts' to indicate the irq type
for this case, we cannot specify the irq number due to the irq number
should be get by gpiod_to_irq(). So the device nodes look weird, since
we should define the index of the interrupt controller instead of the
irq type if the #interrupt_cells is set to 1 according to the
interrupt controller documentation. What do you think about this?
Thanks.
gpio-keys {
compatible = "gpio-keys";
key-volumedown {
label = "Volume Down Key";
linux,code = <KEY_VOLUMEDOWN>;
gpios = <&ap_eic_debounce 2 GPIO_ACTIVE_LOW>;
debounce-interval = <2>;
wakeup-source;
interrupts = <IRQ_TYPE_LEVEL_LOW>;
};
key-volumeup {
label = "Volume Up Key";
linux,code = <KEY_VOLUMEUP>;
gpios = <&pmic_eic 10 GPIO_ACTIVE_HIGH>;
debounce-interval = <2>;
wakeup-source;
interrupts = <IRQ_TYPE_LEVEL_HIGH>;
};
key-power {
label = "Power Key";
linux,code = <KEY_POWER>;
gpios = <&pmic_eic 1 GPIO_ACTIVE_HIGH>;
wakeup-source;
interrupts = <IRQ_TYPE_LEVEL_HIGH>;
};
};
--
Baolin Wang
Best Regards
^ permalink raw reply
* Re: [PATCH v3 01/10] pwm: extend PWM framework with PWM modes
From: Claudiu Beznea @ 2018-02-26 8:11 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, thierry.reding, shc_work, kgene, krzk, linux,
mturquette, sboyd, jani.nikula, joonas.lahtinen, rodrigo.vivi,
airlied, kamil, b.zolnierkie, jdelvare, linux, dmitry.torokhov,
rpurdie, jacek.anaszewski, pavel, mchehab, sean, lee.jones,
daniel.thompson, jingoohan1, milo.kim, robh+dt, mark.rutland,
corbet, nicolas.ferre, alexandre.belloni, linux-pwm
In-Reply-To: <201802250410.reI8wAy2%fengguang.wu@intel.com>
I'll rebase it on latest for-next in next version.
Thank you,
Claudiu Beznea
On 24.02.2018 22:49, kbuild test robot wrote:
> Hi Claudiu,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on pwm/for-next]
> [also build test WARNING on v4.16-rc2 next-20180223]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Claudiu-Beznea/extend-PWM-framework-to-support-PWM-modes/20180225-024011
> base: https://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git for-next
> config: xtensa-allmodconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 7.2.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=xtensa
>
> All warnings (new ones prefixed by >>):
>
>>> drivers//pwm/pwm-sun4i.c:36:0: warning: "PWM_MODE" redefined
> #define PWM_MODE BIT(7)
>
> In file included from drivers//pwm/pwm-sun4i.c:19:0:
> include/linux/pwm.h:40:0: note: this is the location of the previous definition
> #define PWM_MODE(name) BIT(PWM_MODE_##name##_BIT)
>
>
> vim +/PWM_MODE +36 drivers//pwm/pwm-sun4i.c
>
> 09853ce7 Alexandre Belloni 2014-12-17 29
> 09853ce7 Alexandre Belloni 2014-12-17 30 #define PWMCH_OFFSET 15
> 09853ce7 Alexandre Belloni 2014-12-17 31 #define PWM_PRESCAL_MASK GENMASK(3, 0)
> 09853ce7 Alexandre Belloni 2014-12-17 32 #define PWM_PRESCAL_OFF 0
> 09853ce7 Alexandre Belloni 2014-12-17 33 #define PWM_EN BIT(4)
> 09853ce7 Alexandre Belloni 2014-12-17 34 #define PWM_ACT_STATE BIT(5)
> 09853ce7 Alexandre Belloni 2014-12-17 35 #define PWM_CLK_GATING BIT(6)
> 09853ce7 Alexandre Belloni 2014-12-17 @36 #define PWM_MODE BIT(7)
> 09853ce7 Alexandre Belloni 2014-12-17 37 #define PWM_PULSE BIT(8)
> 09853ce7 Alexandre Belloni 2014-12-17 38 #define PWM_BYPASS BIT(9)
> 09853ce7 Alexandre Belloni 2014-12-17 39
>
> :::::: The code at line 36 was first introduced by commit
> :::::: 09853ce7bc1003a490c7ee74a5705d7a7cf16b7d pwm: Add Allwinner SoC support
>
> :::::: TO: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> :::::: CC: Thierry Reding <thierry.reding@gmail.com>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
^ permalink raw reply
* Re: [PATCH v3 0/3] new driver for Valve Steam Controller
From: Benjamin Tissoires @ 2018-02-26 9:50 UTC (permalink / raw)
To: Rodrigo Rivas Costa
Cc: Jiri Kosina, Pierre-Loup A. Griffais, Cameron Gutman,
Clément VUCHENER, lkml, linux-input
In-Reply-To: <20180225185250.10759-1-rodrigorivascosta@gmail.com>
Hi Rodrigo,
On Sun, Feb 25, 2018 at 7:52 PM, Rodrigo Rivas Costa
<rodrigorivascosta@gmail.com> wrote:
> This patchset implements a driver for Valve Steam Controller, based on a
> reverse analysis by myself.
To me, the code looks OK now. I haven't got the time to do a better
review, so giving my:
Acked-by: Bnejamin Tissoires <benjamin.tissoires@redhat.com>
However, before we include it, I'd like to have the ACK from
Pierre-Loup and Clément. They should be more qualified than me to say
if this will interfere with the official Steam client (I think we are
good now, but that's just my opinion).
Cheers,
Benjamin
>
> This is reroll v3, codenamed "lazy lizard". Changes from v2:
> * Use RCU to do the dynamic connec/disconnect of wireless devices. Please,
> to anybody that knows their way around RCU, review.
> * Remove entries in hid-quirks.c as they are no longer needed. This allows
> this module to be blacklisted without side effects.
> * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
> existing use cases (lizard mode). A user-space tool to do that is
> linked.
> * Fully separated axes for joystick and left-pad. As it happens, there are
> people with too many fingers.
> * Add fuzz values for left/right pad axes, they are a little wiggly.
>
> Notable changes from patchset v1:
> * Remove references to USB. Now the interesting interfaces are selected by
> looking for the ones with feature reports.
> * Feature reports buffers are allocated with hid_alloc_report_buf().
> * Feature report length is checked, to avoid overflows in case of
> corrupt/malicius USB devices.
> * Resolution added to the ABS axes.
> * A lot of minor cleanups.
>
> Rodrigo Rivas Costa (3):
> HID: add driver for Valve Steam Controller
> HID: steam: add serial number information.
> HID: steam: add battery device.
>
> drivers/hid/Kconfig | 8 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 4 +
> drivers/hid/hid-steam.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 790 insertions(+)
> create mode 100644 drivers/hid/hid-steam.c
>
> --
> 2.16.2
>
^ permalink raw reply
* Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
From: Jani Nikula @ 2018-02-26 9:57 UTC (permalink / raw)
To: Daniel Thompson, Claudiu Beznea
Cc: mark.rutland, milo.kim, linux-fbdev, sean, devicetree, airlied,
mturquette, kamil, dri-devel, linux-kernel, alexandre.belloni,
pavel, lee.jones, linux-clk, linux-leds, linux-samsung-soc,
shc_work, corbet, linux-doc, linux, krzk, kgene, linux-input,
linux, linux-media, linux-pwm, jdelvare, b.zolnierkie, intel-gfx,
robh+dt, jacek.anaszewski, rodrigo.vivi, mchehab,
linux-arm-kernel, linux-hwmon, jingoohan1, dm
In-Reply-To: <20180222123308.mypx2r7n6o63mj5z@oak.lan>
On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>> were adapted to this change.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>> arch/arm/mach-s3c24xx/mach-rx1950.c | 11 +++++++++--
>> drivers/bus/ts-nbus.c | 2 +-
>> drivers/clk/clk-pwm.c | 3 ++-
>> drivers/gpu/drm/i915/intel_panel.c | 17 ++++++++++++++---
>> drivers/hwmon/pwm-fan.c | 2 +-
>> drivers/input/misc/max77693-haptic.c | 2 +-
>> drivers/input/misc/max8997_haptic.c | 6 +++++-
>> drivers/leds/leds-pwm.c | 5 ++++-
>> drivers/media/rc/ir-rx51.c | 5 ++++-
>> drivers/media/rc/pwm-ir-tx.c | 5 ++++-
>> drivers/video/backlight/lm3630a_bl.c | 4 +++-
>> drivers/video/backlight/lp855x_bl.c | 4 +++-
>> drivers/video/backlight/lp8788_bl.c | 5 ++++-
>> drivers/video/backlight/pwm_bl.c | 11 +++++++++--
>> drivers/video/fbdev/ssd1307fb.c | 3 ++-
>> include/linux/pwm.h | 6 ++++--
>> 16 files changed, 70 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
>> index 2030a6b77a09..696fa25dafd2 100644
>> --- a/drivers/video/backlight/lm3630a_bl.c
>> +++ b/drivers/video/backlight/lm3630a_bl.c
>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>> {
>> unsigned int period = pchip->pdata->pwm_period;
>> unsigned int duty = br * period / br_max;
>> + struct pwm_caps caps = { };
>>
>> - pwm_config(pchip->pwmd, duty, period);
>> + pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>> + pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>
> Well... I admit I've only really looked at the patches that impact
> backlight but dispersing this really odd looking bit twiddling
> throughout the kernel doesn't strike me a great API design.
>
> IMHO callers should not be required to find the first set bit in
> some specially crafted set of capability bits simply to get sane
> default behaviour.
Agreed. IMHO the regular use case becomes rather tedious, ugly, and
error prone.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH v3 0/3] new driver for Valve Steam Controller
From: Clément VUCHENER @ 2018-02-26 11:24 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Rodrigo Rivas Costa, Jiri Kosina, Pierre-Loup A. Griffais,
Cameron Gutman, lkml, linux-input
In-Reply-To: <CAO-hwJ+PDkR4v5r93KJBCqxfm3FTCkrzaOd-K=HxLMwhoAomXQ@mail.gmail.com>
2018-02-26 10:50 GMT+01:00 Benjamin Tissoires <benjamin.tissoires@redhat.com>:
> Hi Rodrigo,
>
> On Sun, Feb 25, 2018 at 7:52 PM, Rodrigo Rivas Costa
> <rodrigorivascosta@gmail.com> wrote:
>> This patchset implements a driver for Valve Steam Controller, based on a
>> reverse analysis by myself.
>
> To me, the code looks OK now. I haven't got the time to do a better
> review, so giving my:
> Acked-by: Bnejamin Tissoires <benjamin.tissoires@redhat.com>
>
> However, before we include it, I'd like to have the ACK from
> Pierre-Loup and Clément. They should be more qualified than me to say
> if this will interfere with the official Steam client (I think we are
> good now, but that's just my opinion).
I am not qualified to speak about the steam client, it has been a long
time since I have a look at how it uses the controller and I think it
changed a lot.
I checked the code and I think you are not handling wireless
connection correctly. If a wireless controller is already connected
when the driver is loaded, you will not receive a connection event and
forget to register the controller. You can send a 0xb4 request with
empty parameters (0xb4, 0x00, ... ) to force the receiver to send its
connection status (as the type 3 event you are already parsing). Since
user-space program may also send this request, you should make sure
the driver works if it receives a (dis)connected event when it is
already (dis)connected.
>
> Cheers,
> Benjamin
>
>>
>> This is reroll v3, codenamed "lazy lizard". Changes from v2:
>> * Use RCU to do the dynamic connec/disconnect of wireless devices. Please,
>> to anybody that knows their way around RCU, review.
>> * Remove entries in hid-quirks.c as they are no longer needed. This allows
>> this module to be blacklisted without side effects.
>> * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
>> existing use cases (lizard mode). A user-space tool to do that is
>> linked.
>> * Fully separated axes for joystick and left-pad. As it happens, there are
>> people with too many fingers.
>> * Add fuzz values for left/right pad axes, they are a little wiggly.
>>
>> Notable changes from patchset v1:
>> * Remove references to USB. Now the interesting interfaces are selected by
>> looking for the ones with feature reports.
>> * Feature reports buffers are allocated with hid_alloc_report_buf().
>> * Feature report length is checked, to avoid overflows in case of
>> corrupt/malicius USB devices.
>> * Resolution added to the ABS axes.
>> * A lot of minor cleanups.
>>
>> Rodrigo Rivas Costa (3):
>> HID: add driver for Valve Steam Controller
>> HID: steam: add serial number information.
>> HID: steam: add battery device.
>>
>> drivers/hid/Kconfig | 8 +
>> drivers/hid/Makefile | 1 +
>> drivers/hid/hid-ids.h | 4 +
>> drivers/hid/hid-steam.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 790 insertions(+)
>> create mode 100644 drivers/hid/hid-steam.c
>>
>> --
>> 2.16.2
>>
^ permalink raw reply
* Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
From: Claudiu Beznea @ 2018-02-26 14:24 UTC (permalink / raw)
To: Jani Nikula, Daniel Thompson
Cc: mark.rutland, milo.kim, linux-fbdev, sean, devicetree, airlied,
mturquette, kamil, dri-devel, linux-kernel, alexandre.belloni,
pavel, lee.jones, linux-clk, linux-leds, linux-samsung-soc,
shc_work, corbet, linux-doc, linux, krzk, kgene, linux-input,
linux, linux-media, linux-pwm, jdelvare, b.zolnierkie, intel-gfx,
robh+dt, jacek.anaszewski, rodrigo.vivi, mchehab,
linux-arm-kernel, linux-hwmon, jingoohan1, dm
In-Reply-To: <87po4s2hve.fsf@intel.com>
On 26.02.2018 11:57, Jani Nikula wrote:
> On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>>> were adapted to this change.
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>> ---
>>> arch/arm/mach-s3c24xx/mach-rx1950.c | 11 +++++++++--
>>> drivers/bus/ts-nbus.c | 2 +-
>>> drivers/clk/clk-pwm.c | 3 ++-
>>> drivers/gpu/drm/i915/intel_panel.c | 17 ++++++++++++++---
>>> drivers/hwmon/pwm-fan.c | 2 +-
>>> drivers/input/misc/max77693-haptic.c | 2 +-
>>> drivers/input/misc/max8997_haptic.c | 6 +++++-
>>> drivers/leds/leds-pwm.c | 5 ++++-
>>> drivers/media/rc/ir-rx51.c | 5 ++++-
>>> drivers/media/rc/pwm-ir-tx.c | 5 ++++-
>>> drivers/video/backlight/lm3630a_bl.c | 4 +++-
>>> drivers/video/backlight/lp855x_bl.c | 4 +++-
>>> drivers/video/backlight/lp8788_bl.c | 5 ++++-
>>> drivers/video/backlight/pwm_bl.c | 11 +++++++++--
>>> drivers/video/fbdev/ssd1307fb.c | 3 ++-
>>> include/linux/pwm.h | 6 ++++--
>>> 16 files changed, 70 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
>>> index 2030a6b77a09..696fa25dafd2 100644
>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>>> {
>>> unsigned int period = pchip->pdata->pwm_period;
>>> unsigned int duty = br * period / br_max;
>>> + struct pwm_caps caps = { };
>>>
>>> - pwm_config(pchip->pwmd, duty, period);
>>> + pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>>> + pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>
>> Well... I admit I've only really looked at the patches that impact
>> backlight but dispersing this really odd looking bit twiddling
>> throughout the kernel doesn't strike me a great API design.
>>
>> IMHO callers should not be required to find the first set bit in
>> some specially crafted set of capability bits simply to get sane
>> default behaviour.
>
> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> error prone.
Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
from your side?
Or, what about using a function like pwm_mode_first() to get the first supported
mode by PWM channel?
Or, would you prefer to solve this inside pwm_config() function, let's say, in
case an invalid mode is passed as argument, to let pwm_config() to choose the
first available PWM mode for PWM channel passed as argument?
Thank you,
Claudiu Beznea
>
> BR,
> Jani.
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH v3 0/3] new driver for Valve Steam Controller
From: Rodrigo Rivas Costa @ 2018-02-26 14:38 UTC (permalink / raw)
To: Clément VUCHENER
Cc: Benjamin Tissoires, Jiri Kosina, Pierre-Loup A. Griffais,
Cameron Gutman, lkml, linux-input
In-Reply-To: <CAM4jgCpX1OOBqCEvqtyx=ftvnuk3zg_jMsFkxFZXxEfZD-ak5Q@mail.gmail.com>
On Mon, Feb 26, 2018 at 12:24:21PM +0100, Clément VUCHENER wrote:
> 2018-02-26 10:50 GMT+01:00 Benjamin Tissoires <benjamin.tissoires@redhat.com>:
> > Hi Rodrigo,
> >
> > On Sun, Feb 25, 2018 at 7:52 PM, Rodrigo Rivas Costa
> > <rodrigorivascosta@gmail.com> wrote:
> >> This patchset implements a driver for Valve Steam Controller, based on a
> >> reverse analysis by myself.
> >
> > To me, the code looks OK now. I haven't got the time to do a better
> > review, so giving my:
> > Acked-by: Bnejamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > However, before we include it, I'd like to have the ACK from
> > Pierre-Loup and Clément. They should be more qualified than me to say
> > if this will interfere with the official Steam client (I think we are
> > good now, but that's just my opinion).
>
> I am not qualified to speak about the steam client, it has been a long
> time since I have a look at how it uses the controller and I think it
> changed a lot.
>
> I checked the code and I think you are not handling wireless
> connection correctly. If a wireless controller is already connected
> when the driver is loaded, you will not receive a connection event and
> forget to register the controller. You can send a 0xb4 request with
> empty parameters (0xb4, 0x00, ... ) to force the receiver to send its
> connection status (as the type 3 event you are already parsing). Since
> user-space program may also send this request, you should make sure
> the driver works if it receives a (dis)connected event when it is
> already (dis)connected.
You are partially right. If a wireless controller is already connected
when the driver is loaded, it will not create the input device. But
eventually it will receive a input or battery report. Then it will know
that it is not yet registered and it will do it. Since the wireless
device sends one battery status per second, that is not a big deal. That
is in patch 3/3, though, without that you will have to touch a button to
get the connection.
The redundant disconnection is handled properly, I think. It will just
call steam_unregister() several times, but that will do nothing if the
input device/battery is not created.
Anyway, I agree that the 0xb4 request is a much better solution, I
didn't know about that command. I'll add it to the driver and check it
from user-land to see if it breaks anything.
Thanks for the tip!
Rodrigo
>
> >
> > Cheers,
> > Benjamin
> >
> >>
> >> This is reroll v3, codenamed "lazy lizard". Changes from v2:
> >> * Use RCU to do the dynamic connec/disconnect of wireless devices. Please,
> >> to anybody that knows their way around RCU, review.
> >> * Remove entries in hid-quirks.c as they are no longer needed. This allows
> >> this module to be blacklisted without side effects.
> >> * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
> >> existing use cases (lizard mode). A user-space tool to do that is
> >> linked.
> >> * Fully separated axes for joystick and left-pad. As it happens, there are
> >> people with too many fingers.
> >> * Add fuzz values for left/right pad axes, they are a little wiggly.
> >>
> >> Notable changes from patchset v1:
> >> * Remove references to USB. Now the interesting interfaces are selected by
> >> looking for the ones with feature reports.
> >> * Feature reports buffers are allocated with hid_alloc_report_buf().
> >> * Feature report length is checked, to avoid overflows in case of
> >> corrupt/malicius USB devices.
> >> * Resolution added to the ABS axes.
> >> * A lot of minor cleanups.
> >>
> >> Rodrigo Rivas Costa (3):
> >> HID: add driver for Valve Steam Controller
> >> HID: steam: add serial number information.
> >> HID: steam: add battery device.
> >>
> >> drivers/hid/Kconfig | 8 +
> >> drivers/hid/Makefile | 1 +
> >> drivers/hid/hid-ids.h | 4 +
> >> drivers/hid/hid-steam.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 790 insertions(+)
> >> create mode 100644 drivers/hid/hid-steam.c
> >>
> >> --
> >> 2.16.2
> >>
^ permalink raw reply
* [PATCH 1/2] input: Add RAVE SP Powerbutton driver
From: Andrey Smirnov @ 2018-02-26 15:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrey Smirnov, linux-input, linux-kernel, Rob Herring,
Mark Rutland, devicetree, Guenter Roeck, Chris Healy, Lucas Stach
Add driver that properly handles input event emitted by RAVE SP
devices.
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
drivers/input/misc/Kconfig | 9 ++++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/rave-sp-pwrbutton.c | 92 ++++++++++++++++++++++++++++++++++
3 files changed, 102 insertions(+)
create mode 100644 drivers/input/misc/rave-sp-pwrbutton.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 62a1312a7387..6a3c753b093b 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -841,4 +841,13 @@ config INPUT_HISI_POWERKEY
To compile this driver as a module, choose M here: the
module will be called hisi_powerkey.
+config INPUT_RAVE_SP_PWRBUTTON
+ tristate "RAVE SP Power button Driver"
+ depends on RAVE_SP_CORE
+ help
+ Say Y here if you want to enable power key reporting from RAVE SP
+
+ To compile this driver as a module, choose M here: the
+ module will be called rave-sp-pwrbutton.
+
endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index a8f61af865aa..6ebeec1f4dd0 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -80,3 +80,4 @@ obj-$(CONFIG_INPUT_WM831X_ON) += wm831x-on.o
obj-$(CONFIG_INPUT_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o
obj-$(CONFIG_INPUT_YEALINK) += yealink.o
obj-$(CONFIG_INPUT_IDEAPAD_SLIDEBAR) += ideapad_slidebar.o
+obj-$(CONFIG_INPUT_RAVE_SP_PWRBUTTON) += rave-sp-pwrbutton.o
diff --git a/drivers/input/misc/rave-sp-pwrbutton.c b/drivers/input/misc/rave-sp-pwrbutton.c
new file mode 100644
index 000000000000..7511062c724b
--- /dev/null
+++ b/drivers/input/misc/rave-sp-pwrbutton.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Power Button driver for RAVE SP
+ *
+ * Copyright (C) 2017 Zodiac Inflight Innovations
+ *
+ */
+
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/rave-sp.h>
+#include <linux/platform_device.h>
+
+#define RAVE_SP_EVNT_BUTTON_PRESS (RAVE_SP_EVNT_BASE + 0x00)
+
+struct rave_sp_power_button {
+ struct input_dev *idev;
+ struct notifier_block nb;
+};
+
+static int rave_sp_power_button_event(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct rave_sp_power_button *pb =
+ container_of(nb, struct rave_sp_power_button, nb);
+ const u8 event = rave_sp_action_unpack_event(action);
+ const u8 value = rave_sp_action_unpack_value(action);
+ struct input_dev *idev = pb->idev;
+
+ if (event == RAVE_SP_EVNT_BUTTON_PRESS) {
+ input_report_key(idev, KEY_POWER, value);
+ input_sync(idev);
+
+ return NOTIFY_STOP;
+ }
+
+ return NOTIFY_DONE;
+}
+
+static int rave_sp_pwrbutton_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct rave_sp_power_button *pb;
+ struct input_dev *idev;
+ int ret;
+
+ pb = devm_kzalloc(dev, sizeof(*pb), GFP_KERNEL);
+ if (!pb)
+ return -ENOMEM;
+
+ idev = devm_input_allocate_device(dev);
+ if (!idev)
+ return -ENOMEM;
+
+ idev->name = pdev->name;
+ idev->dev.parent = dev;
+
+ input_set_capability(idev, EV_KEY, KEY_POWER);
+
+ ret = input_register_device(idev);
+ if (ret)
+ return ret;
+
+ pb->idev = idev;
+ pb->nb.notifier_call = rave_sp_power_button_event;
+ pb->nb.priority = 128;
+
+ return devm_rave_sp_register_event_notifier(dev, &pb->nb);
+}
+
+static const struct of_device_id rave_sp_pwrbutton_of_match[] = {
+ { .compatible = "zii,rave-sp-pwrbutton" },
+ {}
+};
+
+static struct platform_driver rave_sp_pwrbutton_driver = {
+ .probe = rave_sp_pwrbutton_probe,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = rave_sp_pwrbutton_of_match,
+ },
+};
+module_platform_driver(rave_sp_pwrbutton_driver);
+
+MODULE_DEVICE_TABLE(of, rave_sp_pwrbutton_of_match);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>");
+MODULE_AUTHOR("Nikita Yushchenko <nikita.yoush@cogentembedded.com>");
+MODULE_AUTHOR("Andrey Smirnov <andrew.smirnov@gmail.com>");
+MODULE_DESCRIPTION("RAVE SP Power Button driver");
--
2.14.3
^ permalink raw reply related
* [PATCH 2/2] dt-bindings: input: Add binding for RAVE SP input driver
From: Andrey Smirnov @ 2018-02-26 15:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrey Smirnov, linux-input, linux-kernel, Rob Herring,
Mark Rutland, devicetree, Guenter Roeck, Chris Healy, Lucas Stach
In-Reply-To: <20180226154130.25774-1-andrew.smirnov@gmail.com>
Add Device Tree bindings for RAVE SP input drvier - an MFD cell of
parent RAVE SP driver (documented in
Documentation/devicetree/bindings/mfd/zii,rave-sp.txt).
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
.../bindings/input/zii,rave-sp-pwrbutton.txt | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/zii,rave-sp-pwrbutton.txt
diff --git a/Documentation/devicetree/bindings/input/zii,rave-sp-pwrbutton.txt b/Documentation/devicetree/bindings/input/zii,rave-sp-pwrbutton.txt
new file mode 100644
index 000000000000..e14e12d27772
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/zii,rave-sp-pwrbutton.txt
@@ -0,0 +1,24 @@
+Zodiac Inflight Innovations RAVE Supervisory Processor Power Button Bindings
+
+RAVE SP input device is a "MFD cell" device corresponding to power
+button functionality of RAVE Supervisory Processor. It is expected
+that its Device Tree node is specified as a child of the node
+corresponding to the parent RAVE SP device (as documented in
+Documentation/devicetree/bindings/mfd/zii,rave-sp.txt)
+
+Required properties:
+
+- compatible: Should be "zii,rave-sp-pwrbutton"
+
+Example:
+
+ rave-sp {
+ compatible = "zii,rave-sp-rdu1";
+ current-speed = <38400>;
+
+ pwrbutton {
+ compatible = "zii,rave-sp-pwrbutton";
+ status = "okay";
+ };
+ }
+
--
2.14.3
^ permalink raw reply related
* Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
From: Daniel Thompson @ 2018-02-27 10:54 UTC (permalink / raw)
To: Claudiu Beznea
Cc: mark.rutland, milo.kim, linux-fbdev, sean, devicetree, airlied,
mturquette, kamil, dri-devel, linux-kernel, thierry.reding,
alexandre.belloni, pavel, lee.jones, linux-clk, linux-leds,
linux-samsung-soc, shc_work, corbet, linux-doc, linux, krzk,
kgene, linux-input, linux, linux-media, linux-pwm, jdelvare,
b.zolnierkie, intel-gfx, robh+dt, jacek.anaszewski, rodrigo.vivi,
mchehab, linux-arm-kernel, linux-hwmon
In-Reply-To: <3a70b89c-b470-3723-760c-5294d0a75230@microchip.com>
On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
> On 26.02.2018 11:57, Jani Nikula wrote:
> > On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> >> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> >>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> >>> were adapted to this change.
> >>>
> >>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> >>> ---
> >>> arch/arm/mach-s3c24xx/mach-rx1950.c | 11 +++++++++--
> >>> drivers/bus/ts-nbus.c | 2 +-
> >>> drivers/clk/clk-pwm.c | 3 ++-
> >>> drivers/gpu/drm/i915/intel_panel.c | 17 ++++++++++++++---
> >>> drivers/hwmon/pwm-fan.c | 2 +-
> >>> drivers/input/misc/max77693-haptic.c | 2 +-
> >>> drivers/input/misc/max8997_haptic.c | 6 +++++-
> >>> drivers/leds/leds-pwm.c | 5 ++++-
> >>> drivers/media/rc/ir-rx51.c | 5 ++++-
> >>> drivers/media/rc/pwm-ir-tx.c | 5 ++++-
> >>> drivers/video/backlight/lm3630a_bl.c | 4 +++-
> >>> drivers/video/backlight/lp855x_bl.c | 4 +++-
> >>> drivers/video/backlight/lp8788_bl.c | 5 ++++-
> >>> drivers/video/backlight/pwm_bl.c | 11 +++++++++--
> >>> drivers/video/fbdev/ssd1307fb.c | 3 ++-
> >>> include/linux/pwm.h | 6 ++++--
> >>> 16 files changed, 70 insertions(+), 21 deletions(-)
> >>>
> >>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> >>> index 2030a6b77a09..696fa25dafd2 100644
> >>> --- a/drivers/video/backlight/lm3630a_bl.c
> >>> +++ b/drivers/video/backlight/lm3630a_bl.c
> >>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
> >>> {
> >>> unsigned int period = pchip->pdata->pwm_period;
> >>> unsigned int duty = br * period / br_max;
> >>> + struct pwm_caps caps = { };
> >>>
> >>> - pwm_config(pchip->pwmd, duty, period);
> >>> + pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
> >>> + pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
> >>
> >> Well... I admit I've only really looked at the patches that impact
> >> backlight but dispersing this really odd looking bit twiddling
> >> throughout the kernel doesn't strike me a great API design.
> >>
> >> IMHO callers should not be required to find the first set bit in
> >> some specially crafted set of capability bits simply to get sane
> >> default behaviour.
> >
> > Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> > error prone.
>
> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
> from your side?
>
> Or, what about using a function like pwm_mode_first() to get the first supported
> mode by PWM channel?
>
> Or, would you prefer to solve this inside pwm_config() function, let's say, in
> case an invalid mode is passed as argument, to let pwm_config() to choose the
> first available PWM mode for PWM channel passed as argument?
What is it that actually needs solving?
If a driver requests normal mode and the PWM driver cannot support it
why not just return an error an move on.
Put another way, what is the use case for secretly adopting a mode the
caller didn't want? Under what circumstances is this a good thing?
Daniel.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
From: Claudiu Beznea @ 2018-02-27 11:40 UTC (permalink / raw)
To: Daniel Thompson
Cc: Jani Nikula, thierry.reding, shc_work, kgene, krzk, linux,
mturquette, sboyd, joonas.lahtinen, rodrigo.vivi, airlied, kamil,
b.zolnierkie, jdelvare, linux, dmitry.torokhov, rpurdie,
jacek.anaszewski, pavel, mchehab, sean, lee.jones, jingoohan1,
milo.kim, robh+dt, mark.rutland, corbet, nicolas.ferre,
alexandre.belloni, linux-pwm, linux-kernel, linux-ar
In-Reply-To: <20180227105444.lo4pee7vh4we3foq@oak.lan>
On 27.02.2018 12:54, Daniel Thompson wrote:
> On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
>> On 26.02.2018 11:57, Jani Nikula wrote:
>>> On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>>>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>>>>> were adapted to this change.
>>>>>
>>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>>>> ---
>>>>> arch/arm/mach-s3c24xx/mach-rx1950.c | 11 +++++++++--
>>>>> drivers/bus/ts-nbus.c | 2 +-
>>>>> drivers/clk/clk-pwm.c | 3 ++-
>>>>> drivers/gpu/drm/i915/intel_panel.c | 17 ++++++++++++++---
>>>>> drivers/hwmon/pwm-fan.c | 2 +-
>>>>> drivers/input/misc/max77693-haptic.c | 2 +-
>>>>> drivers/input/misc/max8997_haptic.c | 6 +++++-
>>>>> drivers/leds/leds-pwm.c | 5 ++++-
>>>>> drivers/media/rc/ir-rx51.c | 5 ++++-
>>>>> drivers/media/rc/pwm-ir-tx.c | 5 ++++-
>>>>> drivers/video/backlight/lm3630a_bl.c | 4 +++-
>>>>> drivers/video/backlight/lp855x_bl.c | 4 +++-
>>>>> drivers/video/backlight/lp8788_bl.c | 5 ++++-
>>>>> drivers/video/backlight/pwm_bl.c | 11 +++++++++--
>>>>> drivers/video/fbdev/ssd1307fb.c | 3 ++-
>>>>> include/linux/pwm.h | 6 ++++--
>>>>> 16 files changed, 70 insertions(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
>>>>> index 2030a6b77a09..696fa25dafd2 100644
>>>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>>>>> {
>>>>> unsigned int period = pchip->pdata->pwm_period;
>>>>> unsigned int duty = br * period / br_max;
>>>>> + struct pwm_caps caps = { };
>>>>>
>>>>> - pwm_config(pchip->pwmd, duty, period);
>>>>> + pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>>>>> + pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>>>
>>>> Well... I admit I've only really looked at the patches that impact
>>>> backlight but dispersing this really odd looking bit twiddling
>>>> throughout the kernel doesn't strike me a great API design.
>>>>
>>>> IMHO callers should not be required to find the first set bit in
>>>> some specially crafted set of capability bits simply to get sane
>>>> default behaviour.
>>>
>>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
>>> error prone.
>>
>> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
>> from your side?
>>
>> Or, what about using a function like pwm_mode_first() to get the first supported
>> mode by PWM channel?
>>
>> Or, would you prefer to solve this inside pwm_config() function, let's say, in
>> case an invalid mode is passed as argument, to let pwm_config() to choose the
>> first available PWM mode for PWM channel passed as argument?
>
> What is it that actually needs solving?
>
> If a driver requests normal mode and the PWM driver cannot support it
> why not just return an error an move on.
Because, simply, I wasn't aware of what these PWM client drivers needs for.
>
> Put another way, what is the use case for secretly adopting a mode the
> caller didn't want? Under what circumstances is this a good thing?
No one... But I wasn't aware of what the PWM clients needs for from their PWM
controllers. At this moment having BIT(ffs(caps.modes)) instead of
PWM_MODE(NORMAL) is mostly the same since all the driver that has not explicitly
registered PWM caps will use PWM normal mode.
I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK from
your side.
Thank you,
Claudiu Beznea
>
>
> Daniel.
>
^ permalink raw reply
* Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
From: Daniel Thompson @ 2018-02-27 15:38 UTC (permalink / raw)
To: Claudiu Beznea
Cc: mark.rutland, milo.kim, linux-fbdev, sean, devicetree, airlied,
mturquette, kamil, dri-devel, linux-kernel, thierry.reding,
alexandre.belloni, pavel, lee.jones, linux-clk, linux-leds,
linux-samsung-soc, shc_work, corbet, linux-doc, linux, krzk,
kgene, linux-input, linux, linux-media, linux-pwm, jdelvare,
b.zolnierkie, intel-gfx, robh+dt, jacek.anaszewski, rodrigo.vivi,
mchehab, linux-arm-kernel, linux-hwmon
In-Reply-To: <8e1d3b30-3543-56fd-7be6-7fe6edcb40d9@microchip.com>
On Tue, Feb 27, 2018 at 01:40:58PM +0200, Claudiu Beznea wrote:
> On 27.02.2018 12:54, Daniel Thompson wrote:
> > On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
> >> On 26.02.2018 11:57, Jani Nikula wrote:
> >>> On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> >>>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> >>>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> >>>>> were adapted to this change.
> >>>>>
> >>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> >>>>> ---
> >>>>> arch/arm/mach-s3c24xx/mach-rx1950.c | 11 +++++++++--
> >>>>> drivers/bus/ts-nbus.c | 2 +-
> >>>>> drivers/clk/clk-pwm.c | 3 ++-
> >>>>> drivers/gpu/drm/i915/intel_panel.c | 17 ++++++++++++++---
> >>>>> drivers/hwmon/pwm-fan.c | 2 +-
> >>>>> drivers/input/misc/max77693-haptic.c | 2 +-
> >>>>> drivers/input/misc/max8997_haptic.c | 6 +++++-
> >>>>> drivers/leds/leds-pwm.c | 5 ++++-
> >>>>> drivers/media/rc/ir-rx51.c | 5 ++++-
> >>>>> drivers/media/rc/pwm-ir-tx.c | 5 ++++-
> >>>>> drivers/video/backlight/lm3630a_bl.c | 4 +++-
> >>>>> drivers/video/backlight/lp855x_bl.c | 4 +++-
> >>>>> drivers/video/backlight/lp8788_bl.c | 5 ++++-
> >>>>> drivers/video/backlight/pwm_bl.c | 11 +++++++++--
> >>>>> drivers/video/fbdev/ssd1307fb.c | 3 ++-
> >>>>> include/linux/pwm.h | 6 ++++--
> >>>>> 16 files changed, 70 insertions(+), 21 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
> >>>>> index 2030a6b77a09..696fa25dafd2 100644
> >>>>> --- a/drivers/video/backlight/lm3630a_bl.c
> >>>>> +++ b/drivers/video/backlight/lm3630a_bl.c
> >>>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
> >>>>> {
> >>>>> unsigned int period = pchip->pdata->pwm_period;
> >>>>> unsigned int duty = br * period / br_max;
> >>>>> + struct pwm_caps caps = { };
> >>>>>
> >>>>> - pwm_config(pchip->pwmd, duty, period);
> >>>>> + pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
> >>>>> + pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
> >>>>
> >>>> Well... I admit I've only really looked at the patches that impact
> >>>> backlight but dispersing this really odd looking bit twiddling
> >>>> throughout the kernel doesn't strike me a great API design.
> >>>>
> >>>> IMHO callers should not be required to find the first set bit in
> >>>> some specially crafted set of capability bits simply to get sane
> >>>> default behaviour.
> >>>
> >>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> >>> error prone.
> >>
> >> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
> >> from your side?
> >>
> >> Or, what about using a function like pwm_mode_first() to get the first supported
> >> mode by PWM channel?
> >>
> >> Or, would you prefer to solve this inside pwm_config() function, let's say, in
> >> case an invalid mode is passed as argument, to let pwm_config() to choose the
> >> first available PWM mode for PWM channel passed as argument?
> >
> > What is it that actually needs solving?
> >
> > If a driver requests normal mode and the PWM driver cannot support it
> > why not just return an error an move on.
> Because, simply, I wasn't aware of what these PWM client drivers needs for.
I'm afraid you have confused me here.
Didn't you just *add* the whole concept of PWM caps with your patches?
How could any existing call site expect anything except normal mode.
Until now there has been no possiblity to request anything else.
> > Put another way, what is the use case for secretly adopting a mode the
> > caller didn't want? Under what circumstances is this a good thing?
> No one... But I wasn't aware of what the PWM clients needs for from their PWM
> controllers. At this moment having BIT(ffs(caps.modes)) instead of
> PWM_MODE(NORMAL) is mostly the same since all the driver that has not explicitly
> registered PWM caps will use PWM normal mode.
>
> I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK from
> your side.
>
> Thank you,
> Claudiu Beznea
> >
> >
> > Daniel.
> >
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
From: Claudiu Beznea @ 2018-02-27 16:15 UTC (permalink / raw)
To: Daniel Thompson
Cc: mark.rutland, milo.kim, linux-fbdev, sean, devicetree, airlied,
mturquette, kamil, dri-devel, linux-kernel, alexandre.belloni,
pavel, lee.jones, linux-clk, linux-leds, linux-samsung-soc,
shc_work, corbet, linux-doc, linux, krzk, kgene, linux-input,
linux, linux-media, linux-pwm, jdelvare, b.zolnierkie, intel-gfx,
robh+dt, jacek.anaszewski, rodrigo.vivi, mchehab,
linux-arm-kernel, linux-hwmon, jingoohan1, dm
In-Reply-To: <20180227153812.txt2vsdygfnobo33@oak.lan>
On 27.02.2018 17:38, Daniel Thompson wrote:
> On Tue, Feb 27, 2018 at 01:40:58PM +0200, Claudiu Beznea wrote:
>> On 27.02.2018 12:54, Daniel Thompson wrote:
>>> On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
>>>> On 26.02.2018 11:57, Jani Nikula wrote:
>>>>> On Thu, 22 Feb 2018, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>>>>>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>>>>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>>>>>>> were adapted to this change.
>>>>>>>
>>>>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>>>>>> ---
>>>>>>> arch/arm/mach-s3c24xx/mach-rx1950.c | 11 +++++++++--
>>>>>>> drivers/bus/ts-nbus.c | 2 +-
>>>>>>> drivers/clk/clk-pwm.c | 3 ++-
>>>>>>> drivers/gpu/drm/i915/intel_panel.c | 17 ++++++++++++++---
>>>>>>> drivers/hwmon/pwm-fan.c | 2 +-
>>>>>>> drivers/input/misc/max77693-haptic.c | 2 +-
>>>>>>> drivers/input/misc/max8997_haptic.c | 6 +++++-
>>>>>>> drivers/leds/leds-pwm.c | 5 ++++-
>>>>>>> drivers/media/rc/ir-rx51.c | 5 ++++-
>>>>>>> drivers/media/rc/pwm-ir-tx.c | 5 ++++-
>>>>>>> drivers/video/backlight/lm3630a_bl.c | 4 +++-
>>>>>>> drivers/video/backlight/lp855x_bl.c | 4 +++-
>>>>>>> drivers/video/backlight/lp8788_bl.c | 5 ++++-
>>>>>>> drivers/video/backlight/pwm_bl.c | 11 +++++++++--
>>>>>>> drivers/video/fbdev/ssd1307fb.c | 3 ++-
>>>>>>> include/linux/pwm.h | 6 ++++--
>>>>>>> 16 files changed, 70 insertions(+), 21 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
>>>>>>> index 2030a6b77a09..696fa25dafd2 100644
>>>>>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>>>>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>>>>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
>>>>>>> {
>>>>>>> unsigned int period = pchip->pdata->pwm_period;
>>>>>>> unsigned int duty = br * period / br_max;
>>>>>>> + struct pwm_caps caps = { };
>>>>>>>
>>>>>>> - pwm_config(pchip->pwmd, duty, period);
>>>>>>> + pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>>>>>>> + pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>>>>>
>>>>>> Well... I admit I've only really looked at the patches that impact
>>>>>> backlight but dispersing this really odd looking bit twiddling
>>>>>> throughout the kernel doesn't strike me a great API design.
>>>>>>
>>>>>> IMHO callers should not be required to find the first set bit in
>>>>>> some specially crafted set of capability bits simply to get sane
>>>>>> default behaviour.
>>>>>
>>>>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
>>>>> error prone.
>>>>
>>>> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
>>>> from your side?
>>>>
>>>> Or, what about using a function like pwm_mode_first() to get the first supported
>>>> mode by PWM channel?
>>>>
>>>> Or, would you prefer to solve this inside pwm_config() function, let's say, in
>>>> case an invalid mode is passed as argument, to let pwm_config() to choose the
>>>> first available PWM mode for PWM channel passed as argument?
>>>
>>> What is it that actually needs solving?
>>>
>>> If a driver requests normal mode and the PWM driver cannot support it
>>> why not just return an error an move on.
>> Because, simply, I wasn't aware of what these PWM client drivers needs for.
>
> I'm afraid you have confused me here.
>
> Didn't you just *add* the whole concept of PWM caps with your patches?
> How could any existing call site expect anything except normal mode.
> Until now there has been no possiblity to request anything else.
Agree. And agree I was confusing in previous email, sorry about that. And
agree that there was nothing before and everything should work with PWM
normal mode.
When I choose to have BIT(ffs(caps.modes)) instead of PWM_MODE(NORMAL) I
was thinking at having these pwm_config() calls working all the time having
in mind that in future the PWM controllers that these drivers use, might
change in terms of PWM supported modes.
Thank you,
Claudiu Beznea
>
>
>>> Put another way, what is the use case for secretly adopting a mode the
>>> caller didn't want? Under what circumstances is this a good thing?
>> No one... But I wasn't aware of what the PWM clients needs for from their PWM
>> controllers. At this moment having BIT(ffs(caps.modes)) instead of
>> PWM_MODE(NORMAL) is mostly the same since all the driver that has not explicitly
>> registered PWM caps will use PWM normal mode.
>>
>> I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK from
>> your side.
>>
>> Thank you,
>> Claudiu Beznea
>>>
>>>
>>> Daniel.
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH v3] input: bcm5974 - Add driver for Apple Magic Trackpad 2
From: Stephan Mueller @ 2018-02-27 16:37 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov, Henrik Rydberg
Cc: linux-input, marek.wyborski, linux-kernel
In-Reply-To: <2305311.UGlXl2onVb@positron.chronox.de>
Am Sonntag, 21. Januar 2018, 23:06:55 CET schrieb Stephan Müller:
Hi Jiri, Dimity, Henrik,
> Hi,
>
> Changes v3:
> * port to 4.15-rc8
> * small code cleanups (isolation of type casts to functions pertaining
> to the Apple Magic Trackpad 2
> * clean up all checkpatch.pl errors and warnings (except those
> where the patch uses the structure of existing code fragments)
> * updated horizontal and vertical limits to capture start of movements
> in the outer areas of the pad
>
> ---8<---
>
> Add support for Apple Magic Trackpad 2 in bcm5974 (MacBook Tochpad) driver.
> The Magic Trackpad 2 needs to be switched into the finger-reporting-mode,
> just like the other macbook touchpads as well. But the format is different
> to the ones before. The Header is 12 Bytes long and each reported finger
> is additional 9 Bytes. The data order reported by the hardware is
> different as well.
May I ask whether there is an issue in the patch? I have not received any
feedback and would like to inquire about the status of the patch.
I would like to have Touchpad 2 properly supported.
Thanks a lot
Stephan
^ permalink raw reply
* [RFC 0/4] Add support for the Gateworks System Controller
From: Tim Harvey @ 2018-02-28 1:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
linux-input
This series adds support for the Gateworks System Controller used on Gateworks
Laguna, Ventana, and Newport product families.
The GSC is an MSP430 I2C slave controller whose firmware embeds the following
features:
- I/O expander (16 GPIO's emulating a PCA955x)
- EEPROM (enumating AT24)
- RTC (enumating DS1672)
- HWMON
- Interrupt controller with tamper detect, user pushbotton
- Watchdog controller capable of full board power-cycle
- Power Control capable of full board power-cycle
see http://trac.gateworks.com/wiki/gsc for more details
Tim Harvey (4):
dt-bindings: mfd: Add Gateworks System Controller bindings
mfd: add Gateworks System Controller core driver
hwmon: add Gateworks System Controller support
input: misc: Add Gateworks System Controller support
Documentation/devicetree/bindings/mfd/gsc.txt | 69 ++++++
drivers/hwmon/Kconfig | 6 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/gsc-hwmon.c | 299 +++++++++++++++++++++++
drivers/input/misc/Kconfig | 6 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/gsc-input.c | 196 +++++++++++++++
drivers/mfd/Kconfig | 10 +
drivers/mfd/Makefile | 1 +
drivers/mfd/gsc.c | 330 ++++++++++++++++++++++++++
include/linux/mfd/gsc.h | 79 ++++++
11 files changed, 998 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt
create mode 100644 drivers/hwmon/gsc-hwmon.c
create mode 100644 drivers/input/misc/gsc-input.c
create mode 100644 drivers/mfd/gsc.c
create mode 100644 include/linux/mfd/gsc.h
--
2.7.4
^ permalink raw reply
* [RFC 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
From: Tim Harvey @ 2018-02-28 1:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
Cc: linux-hwmon, devicetree, linux-kernel, linux-arm-kernel,
linux-input
In-Reply-To: <1519780874-8558-1-git-send-email-tharvey@gateworks.com>
This patch adds documentation of device-tree bindings for the
Gateworks System Controller (GSC).
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
Documentation/devicetree/bindings/mfd/gsc.txt | 69 +++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt
diff --git a/Documentation/devicetree/bindings/mfd/gsc.txt b/Documentation/devicetree/bindings/mfd/gsc.txt
new file mode 100644
index 0000000..7671347
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/gsc.txt
@@ -0,0 +1,159 @@
+Gateworks System Controller multi-function device
+
+The GSC is a Multifunction I2C slave device with the following submodules:
+- WDT
+- GPIO
+- Pushbutton controller
+- HWMON
+
+Required properties:
+- compatible : Must be "gw,gsc_v1", "gw,gsc_v2", "gw,gsc_v3"
+- reg: I2C address of the device
+- interrupts: interrupt triggered by GSC_IRQ# signal
+- interrupt-parent: Interrupt controller GSC is connected to
+- #interrupt-cells: should be <1>, index of the interrupt within the
+ controller, in accordance with the "one cell" variant of
+ <devicetree/bindings/interrupt-controller/interrupt.txt>
+
+Optional nodes:
+* watchdog:
+The GSC provides a Watchdog monitor which can power cycle the board's
+primary power supply on most board models when tripped.
+
+Required properties:
+- compatible: must be "gw,gsc-watchdog"
+
+* input:
+The GSC provides an input device capable of dispatching Linux Input events
+for user pushbutton events, tamper switch events, etc.
+
+Required properties:
+- compatible: must be "gw,gsc-input"
+
+* hwmon:
+The GSC provides a set of Analog to Digitcal Converter (ADC) pins used for
+temperature and/or voltage monitoring.
+
+Required properties:
+- compatible: must be "gw,gsc-hwmon"
+
+Example:
+
+ gsc: gsc@20 {
+ compatible = "gw,gsc_v2";
+ reg = <0x20>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <4 GPIO_ACTIVE_LOW>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ gsc_input {
+ compatible = "gw,gsc-input";
+ };
+
+ gsc_watchdog {
+ compatible = "gw,gsc-watchdog";
+ };
+
+ gsc_hwmon {
+ compatible = "gw,gsc-hwmon";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hwmon@0 { /* A0: Board Temperature */
+ type = <0>;
+ reg = <0x00>;
+ label = "temp";
+ };
+
+ hwmon@1 { /* A1: Input Voltage */
+ type = <1>;
+ reg = <0x02>;
+ label = "Vin";
+ };
+
+ hwmon@2 { /* A2: 5P0 */
+ type = <1>;
+ reg = <0x0b>;
+ label = "5P0";
+ };
+
+ hwmon@4 { /* A4: 0-5V input */
+ type = <1>;
+ reg = <0x14>;
+ label = "ANL0";
+ };
+
+ hwmon@5 { /* A5: 2P5 PCIe/GigE */
+ type = <1>;
+ reg = <0x23>;
+ label = "2P5";
+ };
+
+ hwmon@6 { /* A6: 1P8 Aud/Vid */
+ type = <1>;
+ reg = <0x1d>;
+ label = "1P8";
+ };
+
+ hwmon@7 { /* A7: GPS */
+ type = <1>;
+ reg = <0x26>;
+ label = "GPS";
+ };
+
+ hwmon@12 { /* A12: VDD_CORE */
+ type = <1>;
+ reg = <0x3>;
+ label = "VDD_CORE";
+ };
+
+ hwmon@13 { /* A13: VDD_SOC */
+ type = <1>;
+ reg = <0x11>;
+ label = "VDD_SOC";
+ };
+
+ hwmon@14 { /* A14: 1P0 PCIe SW */
+ type = <1>;
+ reg = <0x20>;
+ label = "1P0";
+ };
+
+ hwmon@15 { /* fan0 */
+ type = <2>;
+ reg = <0x2c>;
+ label = "fan_50p";
+ };
+
+ hwmon@16 { /* fan1 */
+ type = <2>;
+ reg = <0x2e>;
+ label = "fan_60p";
+ };
+
+ hwmon@17 { /* fan2 */
+ type = <2>;
+ reg = <0x30>;
+ label = "fan_70p";
+ };
+
+ hwmon@18 { /* fan3 */
+ type = <2>;
+ reg = <0x32>;
+ label = "fan_80p";
+ };
+
+ hwmon@19 { /* fan4 */
+ type = <2>;
+ reg = <0x34>;
+ label = "fan_90p";
+ };
+
+ hwmon@20 { /* fan5 */
+ type = <2>;
+ reg = <0x36>;
+ label = "fan_100p";
+ };
+ };
+ };
--
2.7.4
^ permalink raw reply related
* [RFC 2/4] mfd: add Gateworks System Controller core driver
From: Tim Harvey @ 2018-02-28 1:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
linux-input
In-Reply-To: <1519780874-8558-1-git-send-email-tharvey@gateworks.com>
The Gateworks System Controller (GSC) is an I2C slave controller
implemented with an MSP430 micro-controller whose firmware embeds the
following features:
- I/O expander (16 GPIO's) using PCA955x protocol
- Real Time Clock using DS1672 protocol
- User EEPROM using AT24 protocol
- HWMON using custom protocol
- Interrupt controller with tamper detect, user pushbotton
- Watchdog controller capable of full board power-cycle
- Power Control capable of full board power-cycle
see http://trac.gateworks.com/wiki/gsc for more details
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
drivers/mfd/Kconfig | 10 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/gsc.c | 330 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/gsc.h | 79 ++++++++++++
4 files changed, 420 insertions(+)
create mode 100644 drivers/mfd/gsc.c
create mode 100644 include/linux/mfd/gsc.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 1d20a80..16dd486 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -341,6 +341,16 @@ config MFD_EXYNOS_LPASS
Select this option to enable support for Samsung Exynos Low Power
Audio Subsystem.
+config MFD_GSC
+ tristate "Gateworks System Controller"
+ depends on (I2C && OF) || COMPILE_TEST
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
+ Enable support for the Gateworks System Controller found
+ on Gateworks Single Board Computers.
+
config MFD_MC13XXX
tristate
depends on (SPI_MASTER || I2C)
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d9474ad..aede0bc 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_MFD_CROS_EC) += cros_ec_core.o
obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o
obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o
obj-$(CONFIG_MFD_EXYNOS_LPASS) += exynos-lpass.o
+obj-$(CONFIG_MFD_GSC) += gsc.o
rtsx_pci-objs := rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
obj-$(CONFIG_MFD_RTSX_PCI) += rtsx_pci.o
diff --git a/drivers/mfd/gsc.c b/drivers/mfd/gsc.c
new file mode 100644
index 0000000..2fe4174
--- /dev/null
+++ b/drivers/mfd/gsc.c
@@ -0,0 +1,330 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Gateworks Corporation
+ *
+ * The Gateworks System Controller (GSC) is a family of a multi-function
+ * "Power Management and System Companion Device" chips originally designed for
+ * use in Gateworks Single Board Computers. The control interface is I2C,
+ * at 100kbps, with an interrupt.
+ *
+ */
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/gsc.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/*
+ * The GSC suffers from an errata where occasionally during
+ * ADC cycles the chip can NAK i2c transactions. To ensure we have reliable
+ * register access we place retries around register access.
+ */
+#define I2C_RETRIES 3
+
+static int gsc_regmap_regwrite(void *context, unsigned int reg,
+ unsigned int val)
+{
+ struct i2c_client *client = context;
+ int retry, ret;
+
+ for (retry = 0; retry < I2C_RETRIES; retry++) {
+ ret = i2c_smbus_write_byte_data(client, reg, val);
+ /*
+ * -EAGAIN returned when the i2c host controller is busy
+ * -EIO returned when i2c device is busy
+ */
+ if (ret != -EAGAIN && ret != -EIO)
+ break;
+ }
+ if (ret < 0) {
+ dev_err(&client->dev, ">> 0x%02x %d\n", reg, ret);
+ return ret;
+ }
+ dev_dbg(&client->dev, ">> 0x%02x=0x%02x (%d)\n", reg, val, retry);
+
+ return 0;
+}
+
+static int gsc_regmap_regread(void *context, unsigned int reg,
+ unsigned int *val)
+{
+ struct i2c_client *client = context;
+ int retry, ret;
+
+ for (retry = 0; retry < I2C_RETRIES; retry++) {
+ ret = i2c_smbus_read_byte_data(client, reg);
+ /*
+ * -EAGAIN returned when the i2c host controller is busy
+ * -EIO returned when i2c device is busy
+ */
+ if (ret != -EAGAIN && ret != -EIO)
+ break;
+ }
+ if (ret < 0) {
+ dev_err(&client->dev, "<< 0x%02x %d\n", reg, ret);
+ return ret;
+ }
+
+ *val = ret & 0xff;
+ dev_dbg(&client->dev, "<< 0x%02x=0x%02x (%d)\n", reg, *val, retry);
+
+ return 0;
+}
+
+static struct regmap_bus regmap_gsc = {
+ .reg_write = gsc_regmap_regwrite,
+ .reg_read = gsc_regmap_regread,
+};
+
+/*
+ * gsc_powerdown - API to use GSC to power down board for a specific time
+ *
+ * secs - number of seconds to remain powered off
+ */
+static int gsc_powerdown(struct gsc_dev *gsc, unsigned long secs)
+{
+ int ret;
+ unsigned char regs[4];
+
+ dev_info(&gsc->i2c->dev, "GSC powerdown for %ld seconds\n",
+ secs);
+ regs[0] = secs & 0xff;
+ regs[1] = (secs >> 8) & 0xff;
+ regs[2] = (secs >> 16) & 0xff;
+ regs[3] = (secs >> 24) & 0xff;
+ ret = regmap_bulk_write(gsc->regmap, GSC_TIME_ADD, regs, 4);
+
+ return ret;
+}
+
+/*
+ * sysfs hooks
+ */
+static ssize_t gsc_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct gsc_dev *gsc = dev_get_drvdata(dev);
+ const char *name = attr->attr.name;
+ int rz = 0;
+
+ if (strcasecmp(name, "fw_version") == 0)
+ rz = sprintf(buf, "%d\n", gsc->fwver);
+ else if (strcasecmp(name, "fw_crc") == 0)
+ rz = sprintf(buf, "0x%04x\n", gsc->fwcrc);
+
+ return rz;
+}
+
+static ssize_t gsc_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct gsc_dev *gsc = dev_get_drvdata(dev);
+ const char *name = attr->attr.name;
+ int ret;
+
+ if (strcasecmp(name, "powerdown") == 0) {
+ long value;
+
+ ret = kstrtol(buf, 0, &value);
+ if (ret == 0)
+ gsc_powerdown(gsc, value);
+ } else
+ printk(KERN_ERR "invalid name '%s\n", name);
+
+ return count;
+}
+
+
+/*
+ * Create a group of attributes so that we can create and destroy them all
+ * at once.
+ */
+static struct device_attribute attr_fwver =
+ __ATTR(fw_version, 0440, gsc_show, NULL);
+static struct device_attribute attr_fwcrc =
+ __ATTR(fw_crc, 0440, gsc_show, NULL);
+static struct device_attribute attr_pwrdown =
+ __ATTR(powerdown, 0220, NULL, gsc_store);
+
+static struct attribute *gsc_attrs[] = {
+ &attr_fwver.attr,
+ &attr_fwcrc.attr,
+ &attr_pwrdown.attr,
+ NULL,
+};
+
+static struct attribute_group attr_group = {
+ .attrs = gsc_attrs,
+};
+
+static const struct i2c_device_id gsc_i2c_ids[] = {
+ { "gsc_v1", gsc_v1 },
+ { "gsc_v2", gsc_v2 },
+ { "gsc_v3", gsc_v3 },
+ { },
+};
+
+static const struct of_device_id gsc_of_match[] = {
+ { .compatible = "gw,gsc_v1", .data = (void *)gsc_v1 },
+ { .compatible = "gw,gsc_v2", .data = (void *)gsc_v2 },
+ { .compatible = "gw,gsc_v3", .data = (void *)gsc_v3 },
+ { }
+};
+
+static const struct regmap_config gsc_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .cache_type = REGCACHE_NONE,
+ .max_register = 0xf,
+};
+
+static const struct regmap_config gsc_regmap_hwmon_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .cache_type = REGCACHE_NONE,
+ .max_register = 0x37,
+};
+
+static const struct regmap_irq gsc_irqs[] = {
+ REGMAP_IRQ_REG(GSC_IRQ_PB, 0, BIT(GSC_IRQ_PB)),
+ REGMAP_IRQ_REG(GSC_IRQ_KEY_ERASED, 0, BIT(GSC_IRQ_KEY_ERASED)),
+ REGMAP_IRQ_REG(GSC_IRQ_EEPROM_WP, 0, BIT(GSC_IRQ_EEPROM_WP)),
+ REGMAP_IRQ_REG(GSC_IRQ_RESV, 0, BIT(GSC_IRQ_RESV)),
+ REGMAP_IRQ_REG(GSC_IRQ_GPIO, 0, BIT(GSC_IRQ_GPIO)),
+ REGMAP_IRQ_REG(GSC_IRQ_TAMPER, 0, BIT(GSC_IRQ_TAMPER)),
+ REGMAP_IRQ_REG(GSC_IRQ_WDT_TIMEOUT, 0, BIT(GSC_IRQ_WDT_TIMEOUT)),
+ REGMAP_IRQ_REG(GSC_IRQ_SWITCH_HOLD, 0, BIT(GSC_IRQ_SWITCH_HOLD)),
+};
+
+static const struct regmap_irq_chip gsc_irq_chip = {
+ .name = KBUILD_MODNAME,
+ .irqs = gsc_irqs,
+ .num_irqs = ARRAY_SIZE(gsc_irqs),
+ .num_regs = 1,
+ .status_base = GSC_IRQ_STATUS,
+ .mask_base = GSC_IRQ_ENABLE,
+ .mask_invert = true,
+ .ack_base = GSC_IRQ_STATUS,
+ .ack_invert = true,
+};
+
+static int gsc_of_probe(struct device_node *np, struct gsc_dev *gsc)
+{
+ const struct of_device_id *of_id;
+
+ if (!np)
+ return -ENODEV;
+
+ of_id = of_match_device(gsc_of_match, gsc->dev);
+ if (of_id)
+ gsc->type = (enum gsc_type)of_id->data;
+
+ return 0;
+}
+
+static int
+gsc_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct gsc_dev *gsc;
+ int ret;
+ unsigned int reg;
+
+ gsc = devm_kzalloc(dev, sizeof(*gsc), GFP_KERNEL);
+ if (!gsc)
+ return -ENOMEM;
+
+ gsc->dev = &client->dev;
+ gsc->i2c = client;
+ gsc->irq = client->irq;
+ i2c_set_clientdata(client, gsc);
+
+ gsc->regmap = devm_regmap_init(dev, ®map_gsc, client,
+ &gsc_regmap_config);
+ if (IS_ERR(gsc->regmap))
+ return PTR_ERR(gsc->regmap);
+
+ ret = gsc_of_probe(dev->of_node, gsc);
+ if (reg < 0)
+ return ret;
+
+ if (regmap_read(gsc->regmap, GSC_FW_VER, ®))
+ return -EIO;
+ gsc->fwver = reg;
+ regmap_read(gsc->regmap, GSC_FW_CRC, ®);
+ gsc->fwcrc = reg;
+ regmap_read(gsc->regmap, GSC_FW_CRC + 1, ®);
+ gsc->fwcrc |= reg << 8;
+
+ gsc->i2c_hwmon = i2c_new_dummy(client->adapter, GSC_HWMON);
+ if (!gsc->i2c_hwmon) {
+ dev_err(dev, "Failed to allocate I2C device for HWMON\n");
+ return -ENODEV;
+ }
+ i2c_set_clientdata(gsc->i2c_hwmon, gsc);
+
+ gsc->regmap_hwmon = devm_regmap_init(dev, ®map_gsc, gsc->i2c_hwmon,
+ &gsc_regmap_hwmon_config);
+ if (IS_ERR(gsc->regmap_hwmon)) {
+ ret = PTR_ERR(gsc->regmap_hwmon);
+ dev_err(dev, "failed to allocate register map: %d\n", ret);
+ goto err_regmap;
+ }
+
+ ret = devm_regmap_add_irq_chip(dev, gsc->regmap, gsc->irq,
+ IRQF_ONESHOT | IRQF_SHARED |
+ IRQF_TRIGGER_FALLING, 0,
+ &gsc_irq_chip, &gsc->irq_chip_data);
+ if (ret)
+ goto err_regmap;
+
+ dev_info(dev, "Gateworks System Controller v%d: fw v%02d 0x%04x\n",
+ gsc->type, gsc->fwver, gsc->fwcrc);
+
+ /* sysfs hooks */
+ ret = sysfs_create_group(&dev->kobj, &attr_group);
+ if (ret)
+ dev_err(dev, "failed to create sysfs attrs\n");
+
+ ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
+ if (ret)
+ goto err_sysfs;
+
+ return 0;
+
+err_sysfs:
+ sysfs_remove_group(&dev->kobj, &attr_group);
+err_regmap:
+ i2c_unregister_device(gsc->i2c_hwmon);
+
+ return ret;
+}
+
+static int gsc_remove(struct i2c_client *client)
+{
+ sysfs_remove_group(&client->dev.kobj, &attr_group);
+
+ return 0;
+}
+
+static struct i2c_driver gsc_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = of_match_ptr(gsc_of_match),
+ },
+ .probe = gsc_probe,
+ .remove = gsc_remove,
+ .id_table = gsc_i2c_ids,
+};
+
+module_i2c_driver(gsc_driver);
+
+MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
+MODULE_DESCRIPTION("I2C Core interface for GSC");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/gsc.h b/include/linux/mfd/gsc.h
new file mode 100644
index 0000000..2a55e0d
--- /dev/null
+++ b/include/linux/mfd/gsc.h
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Gateworks Corporation
+ */
+#ifndef __LINUX_MFD_GSC_H_
+#define __LINUX_MFD_GSC_H_
+
+/* Device Addresses */
+#define GSC_MISC 0x20
+#define GSC_UPDATE 0x21
+#define GSC_GPIO 0x23
+#define GSC_HWMON 0x29
+#define GSC_EEPROM0 0x50
+#define GSC_EEPROM1 0x51
+#define GSC_EEPROM2 0x52
+#define GSC_EEPROM3 0x53
+#define GSC_RTC 0x68
+
+/* Register offsets */
+#define GSC_CTRL_0 0x00
+#define GSC_CTRL_1 0x01
+#define GSC_TIME 0x02
+#define GSC_TIME_ADD 0x06
+#define GSC_IRQ_STATUS 0x0A
+#define GSC_IRQ_ENABLE 0x0B
+#define GSC_FW_CRC 0x0C
+#define GSC_FW_VER 0x0E
+#define GSC_WP 0x0F
+
+/* Bit definitions */
+#define GSC_CTRL_0_PB_HARD_RESET 0
+#define GSC_CTRL_0_PB_CLEAR_SECURE_KEY 1
+#define GSC_CTRL_0_PB_SOFT_POWER_DOWN 2
+#define GSC_CTRL_0_PB_BOOT_ALTERNATE 3
+#define GSC_CTRL_0_PERFORM_CRC 4
+#define GSC_CTRL_0_TAMPER_DETECT 5
+#define GSC_CTRL_0_SWITCH_HOLD 6
+
+#define GSC_CTRL_1_SLEEP_ENABLE 0
+#define GSC_CTRL_1_ACTIVATE_SLEEP 1
+#define GSC_CTRL_1_LATCH_SLEEP_ADD 2
+#define GSC_CTRL_1_SLEEP_NOWAKEPB 3
+#define GSC_CTRL_1_WDT_TIME 4
+#define GSC_CTRL_1_WDT_ENABLE 5
+#define GSC_CTRL_1_SWITCH_BOOT_ENABLE 6
+#define GSC_CTRL_1_SWITCH_BOOT_CLEAR 7
+
+#define GSC_IRQ_PB 0
+#define GSC_IRQ_KEY_ERASED 1
+#define GSC_IRQ_EEPROM_WP 2
+#define GSC_IRQ_RESV 3
+#define GSC_IRQ_GPIO 4
+#define GSC_IRQ_TAMPER 5
+#define GSC_IRQ_WDT_TIMEOUT 6
+#define GSC_IRQ_SWITCH_HOLD 7
+
+enum gsc_type {
+ gsc_v1 = 1,
+ gsc_v2 = 2,
+ gsc_v3 = 3,
+};
+
+struct gsc_dev {
+ struct device *dev;
+
+ struct i2c_client *i2c; /* 0x20: interrupt controller, WDT */
+ struct i2c_client *i2c_hwmon; /* 0x29: hwmon, fan controller */
+
+ struct regmap *regmap;
+ struct regmap *regmap_hwmon;
+ struct regmap_irq_chip_data *irq_chip_data;
+
+ int irq;
+ enum gsc_type type;
+ unsigned int fwver;
+ unsigned short fwcrc;
+};
+
+#endif /* __LINUX_MFD_GSC_H_ */
--
2.7.4
^ permalink raw reply related
* [RFC 3/4] hwmon: add Gateworks System Controller support
From: Tim Harvey @ 2018-02-28 1:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
linux-input
In-Reply-To: <1519780874-8558-1-git-send-email-tharvey@gateworks.com>
The Gateworks System Controller has a hwmon sub-component that exposes
up to 16 ADC's, some of which are temperature sensors, others which are
voltage inputs. The ADC configuration (register mapping and name) is
configured via device-tree and varies board to board.
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
drivers/hwmon/Kconfig | 6 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/gsc-hwmon.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 306 insertions(+)
create mode 100644 drivers/hwmon/gsc-hwmon.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7ad0176..9cdc3cb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -475,6 +475,12 @@ config SENSORS_F75375S
This driver can also be built as a module. If so, the module
will be called f75375s.
+config SENSORS_GSC
+ tristate "Gateworks System Controller ADC"
+ depends on MFD_GSC
+ help
+ Support for the Gateworks System Controller A/D converters.
+
config SENSORS_MC13783_ADC
tristate "Freescale MC13783/MC13892 ADC"
depends on MFD_MC13XXX
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 0fe489f..835a536 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_SENSORS_G760A) += g760a.o
obj-$(CONFIG_SENSORS_G762) += g762.o
obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
obj-$(CONFIG_SENSORS_GL520SM) += gl520sm.o
+obj-$(CONFIG_SENSORS_GSC) += gsc-hwmon.o
obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
obj-$(CONFIG_SENSORS_HIH6130) += hih6130.o
obj-$(CONFIG_SENSORS_ULTRA45) += ultra45_env.o
diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
new file mode 100644
index 0000000..3e14bea
--- /dev/null
+++ b/drivers/hwmon/gsc-hwmon.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Gateworks Corporation
+ */
+#define DEBUG
+
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/mfd/gsc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+/* map channel to channel info */
+struct gsc_hwmon_ch {
+ u8 reg;
+ char name[32];
+};
+static struct gsc_hwmon_ch gsc_temp_ch[16];
+static struct gsc_hwmon_ch gsc_in_ch[16];
+static struct gsc_hwmon_ch gsc_fan_ch[5];
+
+static int
+gsc_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int ch, long *val)
+{
+ struct gsc_dev *gsc = dev_get_drvdata(dev);
+ int sz, ret;
+ u8 reg;
+ u8 buf[3];
+
+ dev_dbg(dev, "%s type=%d attr=%d ch=%d\n", __func__, type, attr, ch);
+ switch (type) {
+ case hwmon_in:
+ sz = 3;
+ reg = gsc_in_ch[ch].reg;
+ break;
+ case hwmon_temp:
+ sz = 2;
+ reg = gsc_temp_ch[ch].reg;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ ret = regmap_bulk_read(gsc->regmap_hwmon, reg, &buf, sz);
+ if (!ret) {
+ *val = 0;
+ while (sz-- > 0)
+ *val |= (buf[sz] << (8*sz));
+ if ((type == hwmon_temp) && *val > 0x8000)
+ *val -= 0xffff;
+ }
+
+ return ret;
+}
+
+static int
+gsc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int ch, const char **buf)
+{
+ dev_dbg(dev, "%s type=%d attr=%d ch=%d\n", __func__, type, attr, ch);
+ switch (type) {
+ case hwmon_in:
+ case hwmon_temp:
+ case hwmon_fan:
+ switch (attr) {
+ case hwmon_in_label:
+ *buf = gsc_in_ch[ch].name;
+ return 0;
+ break;
+ case hwmon_temp_label:
+ *buf = gsc_temp_ch[ch].name;
+ return 0;
+ break;
+ case hwmon_fan_label:
+ *buf = gsc_fan_ch[ch].name;
+ return 0;
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return -ENOTSUPP;
+}
+
+static int
+gsc_hwmon_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int ch, long val)
+{
+ struct gsc_dev *gsc = dev_get_drvdata(dev);
+ int ret;
+ u8 reg;
+ u8 buf[3];
+
+ dev_dbg(dev, "%s type=%d attr=%d ch=%d\n", __func__, type, attr, ch);
+ switch (type) {
+ case hwmon_fan:
+ buf[0] = val & 0xff;
+ buf[1] = (val >> 8) & 0xff;
+ reg = gsc_fan_ch[ch].reg;
+ ret = regmap_bulk_write(gsc->regmap_hwmon, reg, &buf, 2);
+ break;
+ default:
+ ret = -EOPNOTSUPP;
+ break;
+ }
+
+ return ret;
+}
+
+static umode_t
+gsc_hwmon_is_visible(const void *_data, enum hwmon_sensor_types type, u32 attr,
+ int ch)
+{
+ const struct gsc_dev *gsc = _data;
+ struct device *dev = gsc->dev;
+ umode_t mode = 0;
+
+ switch (type) {
+ case hwmon_fan:
+ if (attr == hwmon_fan_input)
+ mode = (S_IRUGO | S_IWUSR);
+ break;
+ case hwmon_temp:
+ case hwmon_in:
+ mode = S_IRUGO;
+ break;
+ default:
+ break;
+ }
+ dev_dbg(dev, "%s type=%d attr=%d ch=%d mode=0x%x\n", __func__, type,
+ attr, ch, mode);
+
+ return mode;
+}
+
+static u32 gsc_in_config[] = {
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ HWMON_I_INPUT,
+ 0
+};
+static const struct hwmon_channel_info gsc_in = {
+ .type = hwmon_in,
+ .config = gsc_in_config,
+};
+
+static u32 gsc_temp_config[] = {
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ 0
+};
+static const struct hwmon_channel_info gsc_temp = {
+ .type = hwmon_temp,
+ .config = gsc_temp_config,
+};
+
+static u32 gsc_fan_config[] = {
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ 0,
+};
+static const struct hwmon_channel_info gsc_fan = {
+ .type = hwmon_fan,
+ .config = gsc_fan_config,
+};
+
+static const struct hwmon_channel_info *gsc_info[] = {
+ &gsc_temp,
+ &gsc_in,
+ &gsc_fan,
+ NULL
+};
+
+static const struct hwmon_ops gsc_hwmon_ops = {
+ .is_visible = gsc_hwmon_is_visible,
+ .read = gsc_hwmon_read,
+ .read_string = gsc_hwmon_read_string,
+ .write = gsc_hwmon_write,
+};
+
+static const struct hwmon_chip_info gsc_chip_info = {
+ .ops = &gsc_hwmon_ops,
+ .info = gsc_info,
+};
+
+static int gsc_hwmon_probe(struct platform_device *pdev)
+{
+ struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
+ struct device_node *np;
+ struct device *hwmon;
+ int temp_count = 0;
+ int in_count = 0;
+ int fan_count = 0;
+ int ret;
+
+ dev_dbg(&pdev->dev, "%s\n", __func__);
+ np = of_get_next_child(pdev->dev.of_node, NULL);
+ while (np) {
+ u32 reg, type;
+ const char *label;
+
+ of_property_read_u32(np, "reg", ®);
+ of_property_read_u32(np, "type", &type);
+ label = of_get_property(np, "label", NULL);
+ switch(type) {
+ case 0: /* temperature sensor */
+ gsc_temp_config[temp_count] = HWMON_T_INPUT |
+ HWMON_T_LABEL;
+ gsc_temp_ch[temp_count].reg = reg;
+ strncpy(gsc_temp_ch[temp_count].name, label, 32);
+ if (temp_count < ARRAY_SIZE(gsc_temp_config))
+ temp_count++;
+ break;
+ case 1: /* voltage sensor */
+ gsc_in_config[in_count] = HWMON_I_INPUT |
+ HWMON_I_LABEL;
+ gsc_in_ch[in_count].reg = reg;
+ strncpy(gsc_in_ch[in_count].name, label, 32);
+ if (in_count < ARRAY_SIZE(gsc_in_config))
+ in_count++;
+ break;
+ case 2: /* fan controller setpoint */
+ gsc_fan_config[fan_count] = HWMON_F_INPUT |
+ HWMON_F_LABEL;
+ gsc_fan_ch[fan_count].reg = reg;
+ strncpy(gsc_fan_ch[fan_count].name, label, 32);
+ if (fan_count < ARRAY_SIZE(gsc_fan_config))
+ fan_count++;
+ break;
+ }
+ np = of_get_next_child(pdev->dev.of_node, np);
+ }
+ /* terminate list */
+ gsc_in_config[in_count] = 0;
+ gsc_temp_config[temp_count] = 0;
+ gsc_fan_config[fan_count] = 0;
+
+ hwmon = devm_hwmon_device_register_with_info(&pdev->dev,
+ KBUILD_MODNAME, gsc,
+ &gsc_chip_info, NULL);
+ if (IS_ERR(hwmon)) {
+ ret = PTR_ERR(hwmon);
+ dev_err(&pdev->dev, "Unable to register hwmon device: %d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id gsc_hwmon_of_match[] = {
+ { .compatible = "gw,gsc-hwmon", },
+ {}
+};
+
+static struct platform_driver gsc_hwmon_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = gsc_hwmon_of_match,
+ },
+ .probe = gsc_hwmon_probe,
+};
+
+module_platform_driver(gsc_hwmon_driver);
+
+MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
+MODULE_DESCRIPTION("GSC hardware monitor driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* [RFC 4/4] input: misc: Add Gateworks System Controller support
From: Tim Harvey @ 2018-02-28 1:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov
Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
linux-input
In-Reply-To: <1519780874-8558-1-git-send-email-tharvey@gateworks.com>
Add support for dispatching Linux Input events for the various interrupts
the Gateworks System Controller provides.
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
drivers/input/misc/Kconfig | 6 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/gsc-input.c | 196 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 203 insertions(+)
create mode 100644 drivers/input/misc/gsc-input.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 9f082a3..3d18a0e 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -117,6 +117,12 @@ config INPUT_E3X0_BUTTON
To compile this driver as a module, choose M here: the
module will be called e3x0_button.
+config INPUT_GSC
+ tristate "Gateworks System Controller input support"
+ depends on MFD_GSC
+ help
+ Say Y here if you want Gateworks System Controller input support.
+
config INPUT_PCSPKR
tristate "PC Speaker support"
depends on PCSPKR_PLATFORM
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 4b6118d..969debe 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o
obj-$(CONFIG_INPUT_GPIO_BEEPER) += gpio-beeper.o
obj-$(CONFIG_INPUT_GPIO_TILT_POLLED) += gpio_tilt_polled.o
obj-$(CONFIG_INPUT_GPIO_DECODER) += gpio_decoder.o
+obj-$(CONFIG_INPUT_GSC) += gsc-input.o
obj-$(CONFIG_INPUT_HISI_POWERKEY) += hisi_powerkey.o
obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o
obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
new file mode 100644
index 0000000..7cf217c
--- /dev/null
+++ b/drivers/input/misc/gsc-input.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Gateworks Corporation
+ */
+#define DEBUG
+
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/gsc.h>
+
+struct gsc_irq {
+ unsigned int irq;
+ const char *name;
+ unsigned int virq;
+};
+
+static struct gsc_irq gsc_irqs[] = {
+ { GSC_IRQ_PB, "button" },
+ { GSC_IRQ_KEY_ERASED, "key-erased" },
+ { GSC_IRQ_EEPROM_WP, "eeprom-wp" },
+ { GSC_IRQ_TAMPER, "tamper" },
+ { GSC_IRQ_SWITCH_HOLD, "button-held" },
+};
+
+struct gsc_input_info {
+ struct device *dev;
+ struct gsc_dev *gsc;
+ struct input_dev *input;
+
+ int irq;
+ struct work_struct irq_work;
+ struct mutex mutex;
+};
+
+static void gsc_input_irq_work(struct work_struct *work)
+{
+ struct gsc_input_info *info = container_of(work, struct gsc_input_info,
+ irq_work);
+ struct gsc_dev *gsc = info->gsc;
+ int i, ret = 0;
+ int key, sts;
+ struct gsc_irq *gsc_irq = NULL;
+
+ dev_dbg(gsc->dev, "%s irq%d\n", __func__, info->irq);
+ mutex_lock(&info->mutex);
+
+ for (i = 0; i < ARRAY_SIZE(gsc_irqs);i++)
+ if (info->irq == gsc_irqs[i].virq)
+ gsc_irq = &gsc_irqs[i];
+ if (!gsc_irq) {
+ dev_err(info->dev, "interrupt: irq%d occurred\n", info->irq);
+ mutex_unlock(&info->mutex);
+ return;
+ }
+
+ ret = regmap_read(info->gsc->regmap, GSC_IRQ_STATUS, &sts);
+ if (ret) {
+ dev_err(info->dev, "failed to read status register\n");
+ mutex_unlock(&info->mutex);
+ return;
+ }
+
+ key = -1;
+ switch (gsc_irq->virq) {
+ case GSC_IRQ_PB:
+ key = BTN_0;
+ break;
+ case GSC_IRQ_KEY_ERASED:
+ key = BTN_1;
+ break;
+ case GSC_IRQ_EEPROM_WP:
+ key = BTN_2;
+ break;
+ case GSC_IRQ_GPIO:
+ key = BTN_3;
+ break;
+ case GSC_IRQ_TAMPER:
+ key = BTN_4;
+ break;
+ case GSC_IRQ_SWITCH_HOLD:
+ key = BTN_5;
+ break;
+ }
+
+ if (key != -1) {
+ dev_dbg(&info->input->dev, "bit%d: key=0x%03x %s\n",
+ gsc_irq->virq, key, gsc_irq->name);
+ input_report_key(info->input, key, 1);
+ input_report_key(info->input, key, 0);
+ input_sync(info->input);
+ }
+
+ mutex_unlock(&info->mutex);
+}
+
+static irqreturn_t gsc_input_irq(int irq, void *data)
+{
+ struct gsc_input_info *info = data;
+
+ dev_dbg(info->gsc->dev, "%s irq%d\n", __func__, irq);
+ info->irq = irq;
+ schedule_work(&info->irq_work);
+
+ return IRQ_HANDLED;
+}
+
+static int gsc_input_probe(struct platform_device *pdev)
+{
+ struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
+ struct gsc_input_info *info;
+ struct input_dev *input;
+ int ret, i;
+
+ dev_dbg(&pdev->dev, "%s\n", __func__);
+ info = devm_kzalloc(&pdev->dev, sizeof(struct gsc_input_info),
+ GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+ info->dev = &pdev->dev;
+ info->gsc = gsc;
+
+ /* Register input device */
+ input = devm_input_allocate_device(&pdev->dev);
+ if (!input) {
+ dev_err(&pdev->dev, "Can't allocate input device\n");
+ return -ENOMEM;
+ }
+ info->input = input;
+
+ input->name = KBUILD_MODNAME;
+ input->dev.parent = &pdev->dev;
+
+ input_set_capability(input, EV_KEY, BTN_0); /* button */
+ input_set_capability(input, EV_KEY, BTN_1); /* key erased */
+ input_set_capability(input, EV_KEY, BTN_2); /* ee wp */
+ input_set_capability(input, EV_KEY, BTN_3); /* gpio */
+ input_set_capability(input, EV_KEY, BTN_4); /* tamper */
+ input_set_capability(input, EV_KEY, BTN_5); /* button held */
+
+ ret = input_register_device(input);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Can't register input device: %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, gsc);
+ mutex_init(&info->mutex);
+ INIT_WORK(&info->irq_work, gsc_input_irq_work);
+
+ /* Support irq domain */
+ for (i = 0; i < ARRAY_SIZE(gsc_irqs); i++) {
+ struct gsc_irq *gsc_irq = &gsc_irqs[i];
+ int virq;
+
+ virq = regmap_irq_get_virq(gsc->irq_chip_data, gsc_irq->irq);
+ if (virq <= 0)
+ return -EINVAL;
+ gsc_irq->virq = virq;
+
+ ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
+ gsc_input_irq, 0,
+ gsc_irq->name, info);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "failed: irq request (IRQ: %d, error: %d\n)",
+ gsc_irq->irq, ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static const struct of_device_id gsc_input_dt_ids[] = {
+ { .compatible = "gw,gsc-input", },
+ {}
+};
+
+static struct platform_driver gsc_input_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = gsc_input_dt_ids,
+ },
+ .probe = gsc_input_probe,
+};
+
+module_platform_driver(gsc_input_driver);
+
+MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
+MODULE_DESCRIPTION("GSC input driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* Re: [RFC 2/4] mfd: add Gateworks System Controller core driver
From: Randy Dunlap @ 2018-02-28 2:00 UTC (permalink / raw)
To: Tim Harvey, Lee Jones, Rob Herring, Mark Rutland, Mark Brown,
Dmitry Torokhov
Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
linux-input
In-Reply-To: <1519780874-8558-3-git-send-email-tharvey@gateworks.com>
On 02/27/2018 05:21 PM, Tim Harvey wrote:
> The Gateworks System Controller (GSC) is an I2C slave controller
> implemented with an MSP430 micro-controller whose firmware embeds the
> following features:
> - I/O expander (16 GPIO's) using PCA955x protocol
> - Real Time Clock using DS1672 protocol
> - User EEPROM using AT24 protocol
> - HWMON using custom protocol
> - Interrupt controller with tamper detect, user pushbotton
> - Watchdog controller capable of full board power-cycle
> - Power Control capable of full board power-cycle
>
> see http://trac.gateworks.com/wiki/gsc for more details
>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> drivers/mfd/Kconfig | 10 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/gsc.c | 330 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/gsc.h | 79 ++++++++++++
> 4 files changed, 420 insertions(+)
> create mode 100644 drivers/mfd/gsc.c
> create mode 100644 include/linux/mfd/gsc.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1d20a80..16dd486 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -341,6 +341,16 @@ config MFD_EXYNOS_LPASS
> Select this option to enable support for Samsung Exynos Low Power
> Audio Subsystem.
>
> +config MFD_GSC
> + tristate "Gateworks System Controller"
> + depends on (I2C && OF) || COMPILE_TEST
Do both I2C and OF have stubs so that a driver will build when they are
both disabled? I.e., only COMPILE_TEST is enabled?
> + select MFD_CORE
> + select REGMAP_I2C
> + select REGMAP_IRQ
> + help
> + Enable support for the Gateworks System Controller found
> + on Gateworks Single Board Computers.
> +
> config MFD_MC13XXX
> tristate
> depends on (SPI_MASTER || I2C)
thanks,
--
~Randy
^ permalink raw reply
* Re: [RFC 3/4] hwmon: add Gateworks System Controller support
From: Guenter Roeck @ 2018-02-28 2:05 UTC (permalink / raw)
To: Tim Harvey, Lee Jones, Rob Herring, Mark Rutland, Mark Brown,
Dmitry Torokhov
Cc: linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
linux-input
In-Reply-To: <1519780874-8558-4-git-send-email-tharvey@gateworks.com>
On 02/27/2018 05:21 PM, Tim Harvey wrote:
> The Gateworks System Controller has a hwmon sub-component that exposes
> up to 16 ADC's, some of which are temperature sensors, others which are
> voltage inputs. The ADC configuration (register mapping and name) is
> configured via device-tree and varies board to board.
>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> drivers/hwmon/Kconfig | 6 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/gsc-hwmon.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 306 insertions(+)
> create mode 100644 drivers/hwmon/gsc-hwmon.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 7ad0176..9cdc3cb 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -475,6 +475,12 @@ config SENSORS_F75375S
> This driver can also be built as a module. If so, the module
> will be called f75375s.
>
> +config SENSORS_GSC
> + tristate "Gateworks System Controller ADC"
> + depends on MFD_GSC
> + help
> + Support for the Gateworks System Controller A/D converters.
> +
> config SENSORS_MC13783_ADC
> tristate "Freescale MC13783/MC13892 ADC"
> depends on MFD_MC13XXX
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 0fe489f..835a536 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_SENSORS_G760A) += g760a.o
> obj-$(CONFIG_SENSORS_G762) += g762.o
> obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
> obj-$(CONFIG_SENSORS_GL520SM) += gl520sm.o
> +obj-$(CONFIG_SENSORS_GSC) += gsc-hwmon.o
> obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
> obj-$(CONFIG_SENSORS_HIH6130) += hih6130.o
> obj-$(CONFIG_SENSORS_ULTRA45) += ultra45_env.o
> diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
> new file mode 100644
> index 0000000..3e14bea
> --- /dev/null
> +++ b/drivers/hwmon/gsc-hwmon.c
> @@ -0,0 +1,299 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Gateworks Corporation
> + */
> +#define DEBUG
Please no.
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/mfd/gsc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +/* map channel to channel info */
> +struct gsc_hwmon_ch {
> + u8 reg;
> + char name[32];
> +};
> +static struct gsc_hwmon_ch gsc_temp_ch[16];
16 temperature channels ...
> +static struct gsc_hwmon_ch gsc_in_ch[16];
> +static struct gsc_hwmon_ch gsc_fan_ch[5];
> +
> +static int
> +gsc_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int ch, long *val)
> +{
> + struct gsc_dev *gsc = dev_get_drvdata(dev);
> + int sz, ret;
> + u8 reg;
> + u8 buf[3];
> +
> + dev_dbg(dev, "%s type=%d attr=%d ch=%d\n", __func__, type, attr, ch);
> + switch (type) {
> + case hwmon_in:
> + sz = 3;
> + reg = gsc_in_ch[ch].reg;
> + break;
> + case hwmon_temp:
> + sz = 2;
> + reg = gsc_temp_ch[ch].reg;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + ret = regmap_bulk_read(gsc->regmap_hwmon, reg, &buf, sz);
> + if (!ret) {
> + *val = 0;
> + while (sz-- > 0)
> + *val |= (buf[sz] << (8*sz));
> + if ((type == hwmon_temp) && *val > 0x8000)
Excessive [and inconsistent] ( )
Please make this
if (ret)
return ret;
...
return 0;
> + *val -= 0xffff;
> + }
> +
> + return ret;
> +}
> +
> +static int
> +gsc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int ch, const char **buf)
> +{
> + dev_dbg(dev, "%s type=%d attr=%d ch=%d\n", __func__, type, attr, ch);
> + switch (type) {
> + case hwmon_in:
> + case hwmon_temp:
> + case hwmon_fan:
> + switch (attr) {
> + case hwmon_in_label:
> + *buf = gsc_in_ch[ch].name;
> + return 0;
> + break;
> + case hwmon_temp_label:
> + *buf = gsc_temp_ch[ch].name;
> + return 0;
> + break;
> + case hwmon_fan_label:
> + *buf = gsc_fan_ch[ch].name;
> + return 0;
> + break;
return followed by break doesn't make sense.
> + default:
> + break;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -ENOTSUPP;
> +}
> +
> +static int
> +gsc_hwmon_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int ch, long val)
> +{
> + struct gsc_dev *gsc = dev_get_drvdata(dev);
> + int ret;
> + u8 reg;
> + u8 buf[3];
> +
> + dev_dbg(dev, "%s type=%d attr=%d ch=%d\n", __func__, type, attr, ch);
> + switch (type) {
> + case hwmon_fan:
> + buf[0] = val & 0xff;
> + buf[1] = (val >> 8) & 0xff;
> + reg = gsc_fan_ch[ch].reg;
> + ret = regmap_bulk_write(gsc->regmap_hwmon, reg, &buf, 2);
&buf -> buf
> + break;
> + default:
> + ret = -EOPNOTSUPP;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static umode_t
> +gsc_hwmon_is_visible(const void *_data, enum hwmon_sensor_types type, u32 attr,
> + int ch)
> +{
> + const struct gsc_dev *gsc = _data;
> + struct device *dev = gsc->dev;
> + umode_t mode = 0;
> +
> + switch (type) {
> + case hwmon_fan:
> + if (attr == hwmon_fan_input)
> + mode = (S_IRUGO | S_IWUSR);
Unnecessary ( )
> + break;
> + case hwmon_temp:
> + case hwmon_in:
> + mode = S_IRUGO;
> + break;
> + default:
> + break;
> + }
> + dev_dbg(dev, "%s type=%d attr=%d ch=%d mode=0x%x\n", __func__, type,
> + attr, ch, mode);
> +
> + return mode;
> +}
> +
> +static u32 gsc_in_config[] = {
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT,
> + 0
> +};
> +static const struct hwmon_channel_info gsc_in = {
> + .type = hwmon_in,
> + .config = gsc_in_config,
> +};
> +
> +static u32 gsc_temp_config[] = {
> + HWMON_T_INPUT,
> + HWMON_T_INPUT,
> + HWMON_T_INPUT,
> + HWMON_T_INPUT,
> + HWMON_T_INPUT,
> + HWMON_T_INPUT,
> + HWMON_T_INPUT,
> + HWMON_T_INPUT,
> + 0
... but this array only has 8+1 elements. This seems inconsistent.
How about using some defines for array sizes ?
Also, why initialize those arrays ? You are overwriting them below.
You could just use a static size array instead.
I assume it is guaranteed that there is only exactly one instance
of this device in the system. Have you tried what happens if you
declare two instances anyway ? The result must be interesting,
with all those static variables.
> +};
> +static const struct hwmon_channel_info gsc_temp = {
> + .type = hwmon_temp,
> + .config = gsc_temp_config,
> +};
> +
> +static u32 gsc_fan_config[] = {
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + 0,
> +};
The matching gsc_fan_ch has only 5 entries.
> +static const struct hwmon_channel_info gsc_fan = {
> + .type = hwmon_fan,
> + .config = gsc_fan_config,
> +};
> +
> +static const struct hwmon_channel_info *gsc_info[] = {
> + &gsc_temp,
> + &gsc_in,
> + &gsc_fan,
> + NULL
> +};
> +
> +static const struct hwmon_ops gsc_hwmon_ops = {
> + .is_visible = gsc_hwmon_is_visible,
> + .read = gsc_hwmon_read,
> + .read_string = gsc_hwmon_read_string,
> + .write = gsc_hwmon_write,
> +};
> +
> +static const struct hwmon_chip_info gsc_chip_info = {
> + .ops = &gsc_hwmon_ops,
> + .info = gsc_info,
> +};
> +
> +static int gsc_hwmon_probe(struct platform_device *pdev)
> +{
> + struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
> + struct device_node *np;
> + struct device *hwmon;
> + int temp_count = 0;
> + int in_count = 0;
> + int fan_count = 0;
> + int ret;
> +
> + dev_dbg(&pdev->dev, "%s\n", __func__);
You declare local 'dev' variables all over the place, except here,
where it would actually be used multiple times.
Please either declare one here as well, or drop all the others.
> + np = of_get_next_child(pdev->dev.of_node, NULL);
> + while (np) {
> + u32 reg, type;
> + const char *label;
> +
> + of_property_read_u32(np, "reg", ®);
> + of_property_read_u32(np, "type", &type);
> + label = of_get_property(np, "label", NULL);
It must be interesting to see what happens if no 'label' property
is provided. Have you tried ? Also, no validation of 'reg' and 'type' ?
Are you sure ?
> + switch(type) {
> + case 0: /* temperature sensor */
> + gsc_temp_config[temp_count] = HWMON_T_INPUT |
> + HWMON_T_LABEL;
> + gsc_temp_ch[temp_count].reg = reg;
> + strncpy(gsc_temp_ch[temp_count].name, label, 32);
This leaves the string unterminated if it is too long. Have you tested
what happens in this situation ? Consider using strlcpy instead.
Also please use sizeof() instead of '32'.
> + if (temp_count < ARRAY_SIZE(gsc_temp_config))
> + temp_count++;
I would suggest to abort with EINVAL if this happens. Otherwise the last entry
is overwritten, which doesn't make much sense. Also, this accepts up to ARRAY_SIZE()
entries, leaving no termination.
> + break;
> + case 1: /* voltage sensor */
> + gsc_in_config[in_count] = HWMON_I_INPUT |
> + HWMON_I_LABEL;
> + gsc_in_ch[in_count].reg = reg;
So a reg value of 0xXXyy is auto-converted to 0xYY ?
> + strncpy(gsc_in_ch[in_count].name, label, 32);
> + if (in_count < ARRAY_SIZE(gsc_in_config))
> + in_count++;
> + break;
> + case 2: /* fan controller setpoint */
> + gsc_fan_config[fan_count] = HWMON_F_INPUT |
> + HWMON_F_LABEL;
> + gsc_fan_ch[fan_count].reg = reg;
It is going to be interesting to see what happens if there are more than
5 such entries.
> + strncpy(gsc_fan_ch[fan_count].name, label, 32);
> + if (fan_count < ARRAY_SIZE(gsc_fan_config))
> + fan_count++;
> + break;
All other types are silently ignored ?
> + }
> + np = of_get_next_child(pdev->dev.of_node, np);
> + }
> + /* terminate list */
> + gsc_in_config[in_count] = 0;
> + gsc_temp_config[temp_count] = 0;
> + gsc_fan_config[fan_count] = 0;
> +
I would suggest to move above code into a separate function.
> + hwmon = devm_hwmon_device_register_with_info(&pdev->dev,
> + KBUILD_MODNAME, gsc,
> + &gsc_chip_info, NULL);
> + if (IS_ERR(hwmon)) {
> + ret = PTR_ERR(hwmon);
> + dev_err(&pdev->dev, "Unable to register hwmon device: %d\n",
> + ret);
> + return ret;
> + }
The error would be ENOMEM. Is it necessary to report that again ?
> +
> + return 0;
> +}
> +
> +static const struct of_device_id gsc_hwmon_of_match[] = {
> + { .compatible = "gw,gsc-hwmon", },
> + {}
> +};
> +
> +static struct platform_driver gsc_hwmon_driver = {
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = gsc_hwmon_of_match,
> + },
> + .probe = gsc_hwmon_probe,
> +};
> +
> +module_platform_driver(gsc_hwmon_driver);
> +
> +MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
> +MODULE_DESCRIPTION("GSC hardware monitor driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* Re: [RFC 4/4] input: misc: Add Gateworks System Controller support
From: Dmitry Torokhov @ 2018-02-28 4:54 UTC (permalink / raw)
To: Tim Harvey
Cc: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, linux-kernel,
devicetree, linux-arm-kernel, linux-hwmon, linux-input
In-Reply-To: <1519780874-8558-5-git-send-email-tharvey@gateworks.com>
Hi Tim,
On Tue, Feb 27, 2018 at 05:21:14PM -0800, Tim Harvey wrote:
> Add support for dispatching Linux Input events for the various interrupts
> the Gateworks System Controller provides.
>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> drivers/input/misc/Kconfig | 6 ++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/gsc-input.c | 196 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 203 insertions(+)
> create mode 100644 drivers/input/misc/gsc-input.c
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 9f082a3..3d18a0e 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -117,6 +117,12 @@ config INPUT_E3X0_BUTTON
> To compile this driver as a module, choose M here: the
> module will be called e3x0_button.
>
> +config INPUT_GSC
> + tristate "Gateworks System Controller input support"
> + depends on MFD_GSC
> + help
> + Say Y here if you want Gateworks System Controller input support.
> +
"To compile this driver as a module..."
> config INPUT_PCSPKR
> tristate "PC Speaker support"
> depends on PCSPKR_PLATFORM
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 4b6118d..969debe 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o
> obj-$(CONFIG_INPUT_GPIO_BEEPER) += gpio-beeper.o
> obj-$(CONFIG_INPUT_GPIO_TILT_POLLED) += gpio_tilt_polled.o
> obj-$(CONFIG_INPUT_GPIO_DECODER) += gpio_decoder.o
> +obj-$(CONFIG_INPUT_GSC) += gsc-input.o
> obj-$(CONFIG_INPUT_HISI_POWERKEY) += hisi_powerkey.o
> obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o
> obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
> diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
> new file mode 100644
> index 0000000..7cf217c
> --- /dev/null
> +++ b/drivers/input/misc/gsc-input.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Gateworks Corporation
> + */
Let's keep the same // comment block fir the copyright notice as well.
An one-line describing what this driver is would be appreciated too.
> +#define DEBUG
Please no.
> +
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/mfd/gsc.h>
> +
> +struct gsc_irq {
> + unsigned int irq;
> + const char *name;
> + unsigned int virq;
> +};
> +
> +static struct gsc_irq gsc_irqs[] = {
> + { GSC_IRQ_PB, "button" },
> + { GSC_IRQ_KEY_ERASED, "key-erased" },
> + { GSC_IRQ_EEPROM_WP, "eeprom-wp" },
> + { GSC_IRQ_TAMPER, "tamper" },
> + { GSC_IRQ_SWITCH_HOLD, "button-held" },
> +};
> +
> +struct gsc_input_info {
> + struct device *dev;
> + struct gsc_dev *gsc;
> + struct input_dev *input;
> +
> + int irq;
> + struct work_struct irq_work;
> + struct mutex mutex;
> +};
> +
> +static void gsc_input_irq_work(struct work_struct *work)
> +{
> + struct gsc_input_info *info = container_of(work, struct gsc_input_info,
> + irq_work);
> + struct gsc_dev *gsc = info->gsc;
> + int i, ret = 0;
> + int key, sts;
> + struct gsc_irq *gsc_irq = NULL;
> +
> + dev_dbg(gsc->dev, "%s irq%d\n", __func__, info->irq);
> + mutex_lock(&info->mutex);
> +
> + for (i = 0; i < ARRAY_SIZE(gsc_irqs);i++)
> + if (info->irq == gsc_irqs[i].virq)
> + gsc_irq = &gsc_irqs[i];
> + if (!gsc_irq) {
> + dev_err(info->dev, "interrupt: irq%d occurred\n", info->irq);
> + mutex_unlock(&info->mutex);
> + return;
> + }
> +
> + ret = regmap_read(info->gsc->regmap, GSC_IRQ_STATUS, &sts);
Why is this needed? To clear irq? What happens if several events happen
at the same time? Do we lose one of them?
> + if (ret) {
> + dev_err(info->dev, "failed to read status register\n");
> + mutex_unlock(&info->mutex);
> + return;
> + }
> +
> + key = -1;
> + switch (gsc_irq->virq) {
> + case GSC_IRQ_PB:
> + key = BTN_0;
> + break;
> + case GSC_IRQ_KEY_ERASED:
> + key = BTN_1;
> + break;
> + case GSC_IRQ_EEPROM_WP:
> + key = BTN_2;
> + break;
> + case GSC_IRQ_GPIO:
> + key = BTN_3;
> + break;
> + case GSC_IRQ_TAMPER:
> + key = BTN_4;
> + break;
> + case GSC_IRQ_SWITCH_HOLD:
> + key = BTN_5;
Could we provide the mapping in DTS instead of hard-coding them?
> + break;
> + }
> +
> + if (key != -1) {
> + dev_dbg(&info->input->dev, "bit%d: key=0x%03x %s\n",
> + gsc_irq->virq, key, gsc_irq->name);
> + input_report_key(info->input, key, 1);
input_sync();
> + input_report_key(info->input, key, 0);
> + input_sync(info->input);
> + }
> +
> + mutex_unlock(&info->mutex);
> +}
> +
> +static irqreturn_t gsc_input_irq(int irq, void *data)
> +{
> + struct gsc_input_info *info = data;
> +
> + dev_dbg(info->gsc->dev, "%s irq%d\n", __func__, irq);
> + info->irq = irq;
> + schedule_work(&info->irq_work);
Why not use threaded interrupt?
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int gsc_input_probe(struct platform_device *pdev)
> +{
> + struct gsc_dev *gsc = dev_get_drvdata(pdev->dev.parent);
> + struct gsc_input_info *info;
> + struct input_dev *input;
> + int ret, i;
> +
> + dev_dbg(&pdev->dev, "%s\n", __func__);
> + info = devm_kzalloc(&pdev->dev, sizeof(struct gsc_input_info),
> + GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> + info->dev = &pdev->dev;
> + info->gsc = gsc;
> +
> + /* Register input device */
> + input = devm_input_allocate_device(&pdev->dev);
> + if (!input) {
> + dev_err(&pdev->dev, "Can't allocate input device\n");
> + return -ENOMEM;
> + }
> + info->input = input;
> +
> + input->name = KBUILD_MODNAME;
> + input->dev.parent = &pdev->dev;
Not needed - it is set by devm_input_allocate_device().
> +
> + input_set_capability(input, EV_KEY, BTN_0); /* button */
> + input_set_capability(input, EV_KEY, BTN_1); /* key erased */
> + input_set_capability(input, EV_KEY, BTN_2); /* ee wp */
> + input_set_capability(input, EV_KEY, BTN_3); /* gpio */
> + input_set_capability(input, EV_KEY, BTN_4); /* tamper */
> + input_set_capability(input, EV_KEY, BTN_5); /* button held */
> +
> + ret = input_register_device(input);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Can't register input device: %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, gsc);
> + mutex_init(&info->mutex);
> + INIT_WORK(&info->irq_work, gsc_input_irq_work);
> +
> + /* Support irq domain */
> + for (i = 0; i < ARRAY_SIZE(gsc_irqs); i++) {
> + struct gsc_irq *gsc_irq = &gsc_irqs[i];
> + int virq;
> +
> + virq = regmap_irq_get_virq(gsc->irq_chip_data, gsc_irq->irq);
> + if (virq <= 0)
> + return -EINVAL;
> + gsc_irq->virq = virq;
I'd say mapping should be done by MFD piece. You can add interrupts as
resources and fetch them here.
> +
> + ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
> + gsc_input_irq, 0,
> + gsc_irq->name, info);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "failed: irq request (IRQ: %d, error: %d\n)",
> + gsc_irq->irq, ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id gsc_input_dt_ids[] = {
> + { .compatible = "gw,gsc-input", },
> + {}
> +};
> +
> +static struct platform_driver gsc_input_driver = {
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = gsc_input_dt_ids,
> + },
> + .probe = gsc_input_probe,
> +};
> +
> +module_platform_driver(gsc_input_driver);
> +
> +MODULE_AUTHOR("Tim Harvey <tharvey@gateworks.com>");
> +MODULE_DESCRIPTION("GSC input driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] input: Add RAVE SP Powerbutton driver
From: Dmitry Torokhov @ 2018-02-28 4:59 UTC (permalink / raw)
To: Andrey Smirnov
Cc: linux-input, linux-kernel, Rob Herring, Mark Rutland, devicetree,
Guenter Roeck, Chris Healy, Lucas Stach
In-Reply-To: <20180226154130.25774-1-andrew.smirnov@gmail.com>
Hi Andrey,
On Mon, Feb 26, 2018 at 07:41:29AM -0800, Andrey Smirnov wrote:
> Add driver that properly handles input event emitted by RAVE SP
> devices.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> drivers/input/misc/Kconfig | 9 ++++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/rave-sp-pwrbutton.c | 92 ++++++++++++++++++++++++++++++++++
> 3 files changed, 102 insertions(+)
> create mode 100644 drivers/input/misc/rave-sp-pwrbutton.c
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 62a1312a7387..6a3c753b093b 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -841,4 +841,13 @@ config INPUT_HISI_POWERKEY
> To compile this driver as a module, choose M here: the
> module will be called hisi_powerkey.
>
> +config INPUT_RAVE_SP_PWRBUTTON
> + tristate "RAVE SP Power button Driver"
> + depends on RAVE_SP_CORE
> + help
> + Say Y here if you want to enable power key reporting from RAVE SP
> +
> + To compile this driver as a module, choose M here: the
> + module will be called rave-sp-pwrbutton.
> +
> endif
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index a8f61af865aa..6ebeec1f4dd0 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -80,3 +80,4 @@ obj-$(CONFIG_INPUT_WM831X_ON) += wm831x-on.o
> obj-$(CONFIG_INPUT_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o
> obj-$(CONFIG_INPUT_YEALINK) += yealink.o
> obj-$(CONFIG_INPUT_IDEAPAD_SLIDEBAR) += ideapad_slidebar.o
> +obj-$(CONFIG_INPUT_RAVE_SP_PWRBUTTON) += rave-sp-pwrbutton.o
Please put it in [somewhat] alphabetical order (even though ideapad is
out of order).
> diff --git a/drivers/input/misc/rave-sp-pwrbutton.c b/drivers/input/misc/rave-sp-pwrbutton.c
> new file mode 100644
> index 000000000000..7511062c724b
> --- /dev/null
> +++ b/drivers/input/misc/rave-sp-pwrbutton.c
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Power Button driver for RAVE SP
> + *
> + * Copyright (C) 2017 Zodiac Inflight Innovations
> + *
> + */
Let's keep SPDX and copyright in the same // comment block.
> +
> +#include <linux/input.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mfd/rave-sp.h>
> +#include <linux/platform_device.h>
> +
> +#define RAVE_SP_EVNT_BUTTON_PRESS (RAVE_SP_EVNT_BASE + 0x00)
> +
> +struct rave_sp_power_button {
> + struct input_dev *idev;
> + struct notifier_block nb;
> +};
> +
> +static int rave_sp_power_button_event(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct rave_sp_power_button *pb =
> + container_of(nb, struct rave_sp_power_button, nb);
> + const u8 event = rave_sp_action_unpack_event(action);
> + const u8 value = rave_sp_action_unpack_value(action);
> + struct input_dev *idev = pb->idev;
> +
> + if (event == RAVE_SP_EVNT_BUTTON_PRESS) {
> + input_report_key(idev, KEY_POWER, value);
> + input_sync(idev);
> +
> + return NOTIFY_STOP;
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int rave_sp_pwrbutton_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct rave_sp_power_button *pb;
> + struct input_dev *idev;
> + int ret;
> +
> + pb = devm_kzalloc(dev, sizeof(*pb), GFP_KERNEL);
> + if (!pb)
> + return -ENOMEM;
> +
> + idev = devm_input_allocate_device(dev);
> + if (!idev)
> + return -ENOMEM;
> +
> + idev->name = pdev->name;
> + idev->dev.parent = dev;
Not needed - done by devm_input_allocate_device().
> +
> + input_set_capability(idev, EV_KEY, KEY_POWER);
> +
> + ret = input_register_device(idev);
> + if (ret)
> + return ret;
Please call this "error".
> +
> + pb->idev = idev;
> + pb->nb.notifier_call = rave_sp_power_button_event;
> + pb->nb.priority = 128;
> +
> + return devm_rave_sp_register_event_notifier(dev, &pb->nb);
Prefer
error = devm_rave...();
if (error)
return error;
return 0;
> +}
> +
> +static const struct of_device_id rave_sp_pwrbutton_of_match[] = {
> + { .compatible = "zii,rave-sp-pwrbutton" },
> + {}
> +};
> +
> +static struct platform_driver rave_sp_pwrbutton_driver = {
> + .probe = rave_sp_pwrbutton_probe,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = rave_sp_pwrbutton_of_match,
> + },
> +};
> +module_platform_driver(rave_sp_pwrbutton_driver);
> +
> +MODULE_DEVICE_TABLE(of, rave_sp_pwrbutton_of_match);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>");
> +MODULE_AUTHOR("Nikita Yushchenko <nikita.yoush@cogentembedded.com>");
> +MODULE_AUTHOR("Andrey Smirnov <andrew.smirnov@gmail.com>");
> +MODULE_DESCRIPTION("RAVE SP Power Button driver");
> --
> 2.14.3
>
--
Dmitry
^ 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