Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-09 Thread Simon Glass
Hi Marek,

On Tue, 9 Feb 2021 at 01:43, Marek Szyprowski  wrote:
>
> Hi Simon,
>
> On 08.02.2021 18:08, Simon Glass wrote:
> > On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski  
> > wrote:
> >> On 06.02.2021 17:21, Simon Glass wrote:
> >>> On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski  
> >>> wrote:
>  ...
>  Could you give me a bit more hints or point where to start? I've tried
>  to build sandbox, but it fails for v2021.01 release (I've did make
>  sandbox_defconfig && make all). I assume I would need to add adc and
>  adc-keys devices to some sandbox dts and some code triggering and
>  checking the key values, but that's all I know now.
> >>> Well you do need to be able to build sandbox or you will get
> >>> nowhere...what error did you get? Once we understand what went wrong
> >>> we can update the docs. Maybe it is missing a dependency.
> >> $ gcc --version
> >> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
> >> Copyright (C) 2017 Free Software Foundation, Inc.
> >> This is free software; see the source for copying conditions. There is NO
> >> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> >>
> >> $ git checkout v2021.01
> >>
> >> $ make sandbox_defconfig
> >> #
> >> # configuration written to .config
> >> #
> >>
> >> $ make
> >> scripts/kconfig/conf  --syncconfig Kconfig
> >> CFG u-boot.cfg
> >> GEN include/autoconf.mk
> >> GEN include/autoconf.mk.dep
> >> CFGCHK  u-boot.cfg
> >> UPD include/generated/timestamp_autogenerated.h
> >> HOSTCC  tools/mkenvimage.o
> >> HOSTLD  tools/mkenvimage
> >> HOSTCC  tools/fit_image.o
> >> HOSTCC  tools/image-host.o
> >> HOSTCC  tools/dumpimage.o
> >> HOSTLD  tools/dumpimage
> >> HOSTCC  tools/mkimage.o
> >> HOSTLD  tools/mkimage
> >> HOSTLD  tools/fit_info
> >> HOSTLD  tools/fit_check_sign
> >>
> >> ...
> >>
> >> CC  arch/sandbox/cpu/cpu.o
> >> In file included from include/common.h:26:0,
> >>from arch/sandbox/cpu/cpu.c:6:
> >> include/asm/global_data.h:112:58: warning: call-clobbered register used
> >> for global register variable
> >>#define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm ("r9")
> >> ^
> >> include/dm/of.h:86:1: note: in expansion of macro ‘DECLARE_GLOBAL_DATA_PTR’
> >>DECLARE_GLOBAL_DATA_PTR;
> > This is pretty mysterious. Are you sure you are using an x86_64 machine?
>
> I've finally found what caused the issue on my build system. It is
> x86_64 machine, but after some old cross-builds I had an 'asm' symlink
> in u-boot/include directory pointing to arch/arm directory. I'm quite
> surprised that it has not been removed by make clean/distclean/mrproper
> combo.

OK. I wonder if this is after building a U-Boot from 2013? I will send a patch.

Regards,
Simon


Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-09 Thread Marek Szyprowski
Hi Simon,

On 08.02.2021 18:08, Simon Glass wrote:
> On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski  
> wrote:
>> On 06.02.2021 17:21, Simon Glass wrote:
>>> On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski  
>>> wrote:
 ...
 Could you give me a bit more hints or point where to start? I've tried
 to build sandbox, but it fails for v2021.01 release (I've did make
 sandbox_defconfig && make all). I assume I would need to add adc and
 adc-keys devices to some sandbox dts and some code triggering and
 checking the key values, but that's all I know now.
>>> Well you do need to be able to build sandbox or you will get
>>> nowhere...what error did you get? Once we understand what went wrong
>>> we can update the docs. Maybe it is missing a dependency.
>> $ gcc --version
>> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
>> Copyright (C) 2017 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions. There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>
>> $ git checkout v2021.01
>>
>> $ make sandbox_defconfig
>> #
>> # configuration written to .config
>> #
>>
>> $ make
>> scripts/kconfig/conf  --syncconfig Kconfig
>> CFG u-boot.cfg
>> GEN include/autoconf.mk
>> GEN include/autoconf.mk.dep
>> CFGCHK  u-boot.cfg
>> UPD include/generated/timestamp_autogenerated.h
>> HOSTCC  tools/mkenvimage.o
>> HOSTLD  tools/mkenvimage
>> HOSTCC  tools/fit_image.o
>> HOSTCC  tools/image-host.o
>> HOSTCC  tools/dumpimage.o
>> HOSTLD  tools/dumpimage
>> HOSTCC  tools/mkimage.o
>> HOSTLD  tools/mkimage
>> HOSTLD  tools/fit_info
>> HOSTLD  tools/fit_check_sign
>>
>> ...
>>
>> CC  arch/sandbox/cpu/cpu.o
>> In file included from include/common.h:26:0,
>>from arch/sandbox/cpu/cpu.c:6:
>> include/asm/global_data.h:112:58: warning: call-clobbered register used
>> for global register variable
>>#define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm ("r9")
>> ^
>> include/dm/of.h:86:1: note: in expansion of macro ‘DECLARE_GLOBAL_DATA_PTR’
>>DECLARE_GLOBAL_DATA_PTR;
> This is pretty mysterious. Are you sure you are using an x86_64 machine?

I've finally found what caused the issue on my build system. It is 
x86_64 machine, but after some old cross-builds I had an 'asm' symlink 
in u-boot/include directory pointing to arch/arm directory. I'm quite 
surprised that it has not been removed by make clean/distclean/mrproper 
combo.

Best regards

-- 
Marek Szyprowski, PhD
Samsung R Institute Poland



Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-08 Thread Simon Glass
Hi Heinrich,

On Mon, 8 Feb 2021 at 15:18, Heinrich Schuchardt  wrote:
>
> On 2/8/21 6:56 PM, Heinrich Schuchardt wrote:
> > On 2/8/21 6:08 PM, Simon Glass wrote:
> >> HI Marek,
> >>
> >> On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski
> >>  wrote:
> >>>
> >>> Hi Simon,
> >>>
> >>> On 06.02.2021 17:21, Simon Glass wrote:
>  On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski
>   wrote:
> > ...
> > Could you give me a bit more hints or point where to start? I've tried
> > to build sandbox, but it fails for v2021.01 release (I've did make
> > sandbox_defconfig && make all). I assume I would need to add adc and
> > adc-keys devices to some sandbox dts and some code triggering and
> > checking the key values, but that's all I know now.
>  Well you do need to be able to build sandbox or you will get
>  nowhere...what error did you get? Once we understand what went wrong
>  we can update the docs. Maybe it is missing a dependency.
> >>>
> >>> $ gcc --version
> >>> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
> >>> Copyright (C) 2017 Free Software Foundation, Inc.
> >>> This is free software; see the source for copying conditions. There
> >>> is NO
> >>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> >>> PURPOSE.
> >>>
> >>> $ git checkout v2021.01
> >>>
> >>> $ make sandbox_defconfig
> >>> #
> >>> # configuration written to .config
> >>> #
> >>>
> >>> $ make
> >>> scripts/kconfig/conf  --syncconfig Kconfig
> >>> CFG u-boot.cfg
> >>> GEN include/autoconf.mk
> >>> GEN include/autoconf.mk.dep
> >>> CFGCHK  u-boot.cfg
> >>> UPD include/generated/timestamp_autogenerated.h
> >>> HOSTCC  tools/mkenvimage.o
> >>> HOSTLD  tools/mkenvimage
> >>> HOSTCC  tools/fit_image.o
> >>> HOSTCC  tools/image-host.o
> >>> HOSTCC  tools/dumpimage.o
> >>> HOSTLD  tools/dumpimage
> >>> HOSTCC  tools/mkimage.o
> >>> HOSTLD  tools/mkimage
> >>> HOSTLD  tools/fit_info
> >>> HOSTLD  tools/fit_check_sign
> >>>
> >>> ...
> >>>
> >>> CC  arch/sandbox/cpu/cpu.o
> >>> In file included from include/common.h:26:0,
> >>>from arch/sandbox/cpu/cpu.c:6:
> >>> include/asm/global_data.h:112:58: warning: call-clobbered register used
> >>> for global register variable
> >>>#define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm
> >>> ("r9")
> >>> ^
> >>> include/dm/of.h:86:1: note: in expansion of macro
> >>> ‘DECLARE_GLOBAL_DATA_PTR’
> >>>DECLARE_GLOBAL_DATA_PTR;
> >>
> >> This is pretty mysterious. Are you sure you are using an x86_64 machine?
> >
> > r9 relates to ARMv7.
> >
> > You have to unset CROSS_COMPILE when you build the sandbox.
> >
> > The sandbox runs fine on aarch64.
> >
> > There are a bunch of errors currently when building on 32bit
> > architectures. Simon has a lot of patches pending.
>
> Hello Simon,
>
> it was your patch
>
> 091401131085 ("command: Remove the cmd_tbl_t typedef") 2020-05-10
>
> that broke building the sandbox on ARMv7.

Oh dear.

>
> Once your 32bit corrections are merged we should try to add
> cross-compiling the sandbox for aarch64 and armv7 to Gitlab CI.

Yes, what we don't test doesn't work :-)

Regards,
Simon


Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-08 Thread Heinrich Schuchardt

On 2/8/21 6:56 PM, Heinrich Schuchardt wrote:

On 2/8/21 6:08 PM, Simon Glass wrote:

HI Marek,

On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski
 wrote:


Hi Simon,

On 06.02.2021 17:21, Simon Glass wrote:

On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski
 wrote:

...
Could you give me a bit more hints or point where to start? I've tried
to build sandbox, but it fails for v2021.01 release (I've did make
sandbox_defconfig && make all). I assume I would need to add adc and
adc-keys devices to some sandbox dts and some code triggering and
checking the key values, but that's all I know now.

Well you do need to be able to build sandbox or you will get
nowhere...what error did you get? Once we understand what went wrong
we can update the docs. Maybe it is missing a dependency.


$ gcc --version
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There
is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.

$ git checkout v2021.01

$ make sandbox_defconfig
#
# configuration written to .config
#

$ make
scripts/kconfig/conf  --syncconfig Kconfig
    CFG u-boot.cfg
    GEN include/autoconf.mk
    GEN include/autoconf.mk.dep
    CFGCHK  u-boot.cfg
    UPD include/generated/timestamp_autogenerated.h
    HOSTCC  tools/mkenvimage.o
    HOSTLD  tools/mkenvimage
    HOSTCC  tools/fit_image.o
    HOSTCC  tools/image-host.o
    HOSTCC  tools/dumpimage.o
    HOSTLD  tools/dumpimage
    HOSTCC  tools/mkimage.o
    HOSTLD  tools/mkimage
    HOSTLD  tools/fit_info
    HOSTLD  tools/fit_check_sign

...

    CC  arch/sandbox/cpu/cpu.o
In file included from include/common.h:26:0,
   from arch/sandbox/cpu/cpu.c:6:
include/asm/global_data.h:112:58: warning: call-clobbered register used
for global register variable
   #define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm
("r9")
    ^
include/dm/of.h:86:1: note: in expansion of macro
‘DECLARE_GLOBAL_DATA_PTR’
   DECLARE_GLOBAL_DATA_PTR;


This is pretty mysterious. Are you sure you are using an x86_64 machine?


r9 relates to ARMv7.

You have to unset CROSS_COMPILE when you build the sandbox.

The sandbox runs fine on aarch64.

There are a bunch of errors currently when building on 32bit
architectures. Simon has a lot of patches pending.


Hello Simon,

it was your patch

091401131085 ("command: Remove the cmd_tbl_t typedef") 2020-05-10

that broke building the sandbox on ARMv7.

Once your 32bit corrections are merged we should try to add
cross-compiling the sandbox for aarch64 and armv7 to Gitlab CI.

Best regards

Heinrich


Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-08 Thread Heinrich Schuchardt

On 2/8/21 6:08 PM, Simon Glass wrote:

HI Marek,

On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski  wrote:


Hi Simon,

On 06.02.2021 17:21, Simon Glass wrote:

On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski  wrote:

...
Could you give me a bit more hints or point where to start? I've tried
to build sandbox, but it fails for v2021.01 release (I've did make
sandbox_defconfig && make all). I assume I would need to add adc and
adc-keys devices to some sandbox dts and some code triggering and
checking the key values, but that's all I know now.

Well you do need to be able to build sandbox or you will get
nowhere...what error did you get? Once we understand what went wrong
we can update the docs. Maybe it is missing a dependency.


$ gcc --version
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ git checkout v2021.01

$ make sandbox_defconfig
#
# configuration written to .config
#

$ make
scripts/kconfig/conf  --syncconfig Kconfig
CFG u-boot.cfg
GEN include/autoconf.mk
GEN include/autoconf.mk.dep
CFGCHK  u-boot.cfg
UPD include/generated/timestamp_autogenerated.h
HOSTCC  tools/mkenvimage.o
HOSTLD  tools/mkenvimage
HOSTCC  tools/fit_image.o
HOSTCC  tools/image-host.o
HOSTCC  tools/dumpimage.o
HOSTLD  tools/dumpimage
HOSTCC  tools/mkimage.o
HOSTLD  tools/mkimage
HOSTLD  tools/fit_info
HOSTLD  tools/fit_check_sign

...

CC  arch/sandbox/cpu/cpu.o
In file included from include/common.h:26:0,
   from arch/sandbox/cpu/cpu.c:6:
include/asm/global_data.h:112:58: warning: call-clobbered register used
for global register variable
   #define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm ("r9")
^
include/dm/of.h:86:1: note: in expansion of macro ‘DECLARE_GLOBAL_DATA_PTR’
   DECLARE_GLOBAL_DATA_PTR;


This is pretty mysterious. Are you sure you are using an x86_64 machine?


r9 relates to ARMv7.

You have to unset CROSS_COMPILE when you build the sandbox.

The sandbox runs fine on aarch64.

There are a bunch of errors currently when building on 32bit
architectures. Simon has a lot of patches pending.

Best regards

Heinrich


Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-08 Thread Simon Glass
HI Marek,

On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski  wrote:
>
> Hi Simon,
>
> On 06.02.2021 17:21, Simon Glass wrote:
> > On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski  
> > wrote:
> >> ...
> >> Could you give me a bit more hints or point where to start? I've tried
> >> to build sandbox, but it fails for v2021.01 release (I've did make
> >> sandbox_defconfig && make all). I assume I would need to add adc and
> >> adc-keys devices to some sandbox dts and some code triggering and
> >> checking the key values, but that's all I know now.
> > Well you do need to be able to build sandbox or you will get
> > nowhere...what error did you get? Once we understand what went wrong
> > we can update the docs. Maybe it is missing a dependency.
>
> $ gcc --version
> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
> Copyright (C) 2017 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> $ git checkout v2021.01
>
> $ make sandbox_defconfig
> #
> # configuration written to .config
> #
>
> $ make
> scripts/kconfig/conf  --syncconfig Kconfig
>CFG u-boot.cfg
>GEN include/autoconf.mk
>GEN include/autoconf.mk.dep
>CFGCHK  u-boot.cfg
>UPD include/generated/timestamp_autogenerated.h
>HOSTCC  tools/mkenvimage.o
>HOSTLD  tools/mkenvimage
>HOSTCC  tools/fit_image.o
>HOSTCC  tools/image-host.o
>HOSTCC  tools/dumpimage.o
>HOSTLD  tools/dumpimage
>HOSTCC  tools/mkimage.o
>HOSTLD  tools/mkimage
>HOSTLD  tools/fit_info
>HOSTLD  tools/fit_check_sign
>
> ...
>
>CC  arch/sandbox/cpu/cpu.o
> In file included from include/common.h:26:0,
>   from arch/sandbox/cpu/cpu.c:6:
> include/asm/global_data.h:112:58: warning: call-clobbered register used
> for global register variable
>   #define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm ("r9")
>^
> include/dm/of.h:86:1: note: in expansion of macro ‘DECLARE_GLOBAL_DATA_PTR’
>   DECLARE_GLOBAL_DATA_PTR;

This is pretty mysterious. Are you sure you are using an x86_64 machine?

Regards,
Simon


Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-08 Thread Marek Szyprowski
Hi Simon,

On 06.02.2021 17:21, Simon Glass wrote:
> On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski  
> wrote:
>> ...
>> Could you give me a bit more hints or point where to start? I've tried
>> to build sandbox, but it fails for v2021.01 release (I've did make
>> sandbox_defconfig && make all). I assume I would need to add adc and
>> adc-keys devices to some sandbox dts and some code triggering and
>> checking the key values, but that's all I know now.
> Well you do need to be able to build sandbox or you will get
> nowhere...what error did you get? Once we understand what went wrong
> we can update the docs. Maybe it is missing a dependency.

$ gcc --version
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ git checkout v2021.01

$ make sandbox_defconfig
#
# configuration written to .config
#

$ make
scripts/kconfig/conf  --syncconfig Kconfig
   CFG u-boot.cfg
   GEN include/autoconf.mk
   GEN include/autoconf.mk.dep
   CFGCHK  u-boot.cfg
   UPD include/generated/timestamp_autogenerated.h
   HOSTCC  tools/mkenvimage.o
   HOSTLD  tools/mkenvimage
   HOSTCC  tools/fit_image.o
   HOSTCC  tools/image-host.o
   HOSTCC  tools/dumpimage.o
   HOSTLD  tools/dumpimage
   HOSTCC  tools/mkimage.o
   HOSTLD  tools/mkimage
   HOSTLD  tools/fit_info
   HOSTLD  tools/fit_check_sign

...

   CC  arch/sandbox/cpu/cpu.o
In file included from include/common.h:26:0,
  from arch/sandbox/cpu/cpu.c:6:
include/asm/global_data.h:112:58: warning: call-clobbered register used 
for global register variable
  #define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm ("r9")
   ^
include/dm/of.h:86:1: note: in expansion of macro ‘DECLARE_GLOBAL_DATA_PTR’
  DECLARE_GLOBAL_DATA_PTR;
  ^~~
In file included from arch/sandbox/cpu/cpu.c:18:0:
./arch/sandbox/include/asm/state.h:98:30: error: 
‘CONFIG_SANDBOX_SPI_MAX_BUS’ undeclared here (not in a function); did 
you mean ‘CONFIG_SANDBOX_SPI’?
   struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
   ^~
   CONFIG_SANDBOX_SPI
./arch/sandbox/include/asm/state.h:99:7: error: 
‘CONFIG_SANDBOX_SPI_MAX_CS’ undeclared here (not in a function); did you 
mean ‘CONFIG_SANDBOX_SPI_MAX_BUS’?
   [CONFIG_SANDBOX_SPI_MAX_CS];
    ^
    CONFIG_SANDBOX_SPI_MAX_BUS
arch/sandbox/cpu/cpu.c: In function ‘is_in_sandbox_mem’:
arch/sandbox/cpu/cpu.c:83:41: error: ‘volatile struct arch_global_data’ 
has no member named ‘ram_buf’
   return (const uint8_t *)ptr >= gd->arch.ram_buf &&
  ^
arch/sandbox/cpu/cpu.c:84:34: error: ‘volatile struct arch_global_data’ 
has no member named ‘ram_buf’
    (const uint8_t *)ptr < gd->arch.ram_buf + gd->ram_size;
   ^
arch/sandbox/cpu/cpu.c: At top level:
arch/sandbox/cpu/cpu.c:97:7: error: redefinition of ‘phys_to_virt’
  void *phys_to_virt(phys_addr_t paddr)
    ^~~~
In file included from include/asm/io.h:495:0,
  from arch/sandbox/cpu/cpu.c:15:
include/asm-generic/io.h:30:21: note: previous definition of 
‘phys_to_virt’ was here
  static inline void *phys_to_virt(phys_addr_t paddr)
  ^~~~
arch/sandbox/cpu/cpu.c: In function ‘phys_to_virt’:
arch/sandbox/cpu/cpu.c:104:27: error: ‘volatile struct arch_global_data’ 
has no member named ‘ram_buf’
    return (void *)(gd->arch.ram_buf + paddr);
    ^
In file included from include/linux/posix_types.h:4:0,
  from include/linux/types.h:4,
  from include/time.h:7,
  from include/common.h:18,
  from arch/sandbox/cpu/cpu.c:6:
include/linux/stddef.h:17:33: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-cast]
  #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
  ^
include/linux/kernel.h:274:29: note: in expansion of macro ‘offsetof’
   (type *)( (char *)__mptr - offsetof(type,member) );})
  ^~~~
include/linux/list.h:327:2: note: in expansion of macro ‘container_of’
   container_of(ptr, type, member)
   ^~~~
include/linux/list.h:424:13: note: in expansion of macro ‘list_entry’
   for (pos = list_entry((head)->next, typeof(*pos), member); \
  ^~
arch/sandbox/cpu/cpu.c:111:2: note: in expansion of macro 
‘list_for_each_entry’
   list_for_each_entry(mentry, >mapmem_head, sibling_node) {
   ^~~
include/linux/stddef.h:17:33: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-cast]
  #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 

Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-06 Thread Simon Glass
Hi Marek,

On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski  wrote:
>
> Hi Simon,
>
> On 01.02.2021 21:38, Simon Glass wrote:
> > On Tue, 26 Jan 2021 at 06:03, Heinrich Schuchardt  
> > wrote:
> >> On 26.01.21 12:25, Marek Szyprowski wrote:
> >>> On 26.01.2021 12:10, Heinrich Schuchardt wrote:
>  On 1/26/21 10:50 AM, Marek Szyprowski wrote:
> > Add a simple Analog to Digital Converter device based button driver.
> > This
> > driver binds to the 'adc-keys' device tree node.
> >
> > Signed-off-by: Marek Szyprowski 
> > ---
> >drivers/button/Kconfig  |   8 ++
> >drivers/button/Makefile |   1 +
> >drivers/button/button-adc.c | 156 
> > 
> >3 files changed, 165 insertions(+)
> >create mode 100644 drivers/button/button-adc.c
> >
> > diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> > index 6b3ec7e55d..6db3c5e93a 100644
> > --- a/drivers/button/Kconfig
> > +++ b/drivers/button/Kconfig
> > @@ -9,6 +9,14 @@ config BUTTON
> >  can provide access to board-specific buttons. Use of the
> > device tree
> >  for configuration is encouraged.
> >
> > +config BUTTON_ADC
> > +bool "Button adc"
> > +depends on BUTTON
> > +help
> > +  Enable support for buttons which are connected to Analog to
> > Digital
> > +  Converter device. The ADC driver must use driver model.
> > Buttons are
> > +  configured using the device tree.
> > +
> >config BUTTON_GPIO
> >bool "Button gpio"
> >depends on BUTTON
> > diff --git a/drivers/button/Makefile b/drivers/button/Makefile
> > index fcc10ebe8d..bbd18af149 100644
> > --- a/drivers/button/Makefile
> > +++ b/drivers/button/Makefile
> > @@ -3,4 +3,5 @@
> ># Copyright (C) 2020 Philippe Reynes 
> >
> >obj-$(CONFIG_BUTTON) += button-uclass.o
> > +obj-$(CONFIG_BUTTON_ADC) += button-adc.o
> >obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
> > diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c
> > new file mode 100644
> > index 00..1901d59a0e
> > --- /dev/null
> > +++ b/drivers/button/button-adc.c
> > @@ -0,0 +1,156 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2021 Samsung Electronics Co., Ltd.
> > + *http://www.samsung.com
> > + * Author: Marek Szyprowski 
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/**
> > + * struct button_adc_priv - private data for button-adc driver.
> > + *
> > + * @adc: Analog to Digital Converter device to which button is
> > connected.
> > + * @channel: channel of the ADC device to probe the button state.
> > + * @min: minimal raw ADC sample value to consider button as pressed.
> > + * @max: maximal raw ADC sample value to consider button as pressed.
> > + */
> > +struct button_adc_priv {
> > +struct udevice *adc;
> > +int channel;
> > +int min;
> > +int max;
> > +};
> > +
> > +static enum button_state_t button_adc_get_state(struct udevice *dev)
> > +{
> > +struct button_adc_priv *priv = dev_get_priv(dev);
> > +unsigned int val;
> > +int ret;
> > +
> > +ret = adc_start_channel(priv->adc, priv->channel);
> > +if (ret)
> > +return ret;
> > +
> > +ret = adc_channel_data(priv->adc, priv->channel, );
> > +if (ret)
> > +return ret;
> > +
> > +if (ret == 0)
> > +return (val >= priv->min && val < priv->max) ?
> > +BUTTON_ON : BUTTON_OFF;
> > +
> > +return ret;
> > +}
> > +
> > +static int button_adc_probe(struct udevice *dev)
> > +{
> > +struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> > +struct button_adc_priv *priv = dev_get_priv(dev);
> > +struct ofnode_phandle_args args;
> > +u32 treshold, up_treshold, t;
> > +unsigned int mask;
> > +ofnode node;
> > +int ret, vdd;
> > +
> > +/* Ignore the top-level button node */
> > +if (!uc_plat->label)
> > +return 0;
> > +
> > +ret = dev_read_phandle_with_args(dev->parent, "io-channels",
> > + "#io-channel-cells", 0, 0, );
> > +if (ret)
> > +return ret;
> > +
> > +ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node,
> > >adc);
> > +if (ret)
> > +return ret;
> > +
> > +ret = ofnode_read_u32(dev_ofnode(dev->parent),
> > +  "keyup-threshold-microvolt", _treshold);
> > +if (ret)
> > +return 

Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-04 Thread Marek Szyprowski
Hi Simon,

On 01.02.2021 21:38, Simon Glass wrote:
> On Tue, 26 Jan 2021 at 06:03, Heinrich Schuchardt  wrote:
>> On 26.01.21 12:25, Marek Szyprowski wrote:
>>> On 26.01.2021 12:10, Heinrich Schuchardt wrote:
 On 1/26/21 10:50 AM, Marek Szyprowski wrote:
> Add a simple Analog to Digital Converter device based button driver.
> This
> driver binds to the 'adc-keys' device tree node.
>
> Signed-off-by: Marek Szyprowski 
> ---
>drivers/button/Kconfig  |   8 ++
>drivers/button/Makefile |   1 +
>drivers/button/button-adc.c | 156 
>3 files changed, 165 insertions(+)
>create mode 100644 drivers/button/button-adc.c
>
> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> index 6b3ec7e55d..6db3c5e93a 100644
> --- a/drivers/button/Kconfig
> +++ b/drivers/button/Kconfig
> @@ -9,6 +9,14 @@ config BUTTON
>  can provide access to board-specific buttons. Use of the
> device tree
>  for configuration is encouraged.
>
> +config BUTTON_ADC
> +bool "Button adc"
> +depends on BUTTON
> +help
> +  Enable support for buttons which are connected to Analog to
> Digital
> +  Converter device. The ADC driver must use driver model.
> Buttons are
> +  configured using the device tree.
> +
>config BUTTON_GPIO
>bool "Button gpio"
>depends on BUTTON
> diff --git a/drivers/button/Makefile b/drivers/button/Makefile
> index fcc10ebe8d..bbd18af149 100644
> --- a/drivers/button/Makefile
> +++ b/drivers/button/Makefile
> @@ -3,4 +3,5 @@
># Copyright (C) 2020 Philippe Reynes 
>
>obj-$(CONFIG_BUTTON) += button-uclass.o
> +obj-$(CONFIG_BUTTON_ADC) += button-adc.o
>obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
> diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c
> new file mode 100644
> index 00..1901d59a0e
> --- /dev/null
> +++ b/drivers/button/button-adc.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 Samsung Electronics Co., Ltd.
> + *http://www.samsung.com
> + * Author: Marek Szyprowski 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/**
> + * struct button_adc_priv - private data for button-adc driver.
> + *
> + * @adc: Analog to Digital Converter device to which button is
> connected.
> + * @channel: channel of the ADC device to probe the button state.
> + * @min: minimal raw ADC sample value to consider button as pressed.
> + * @max: maximal raw ADC sample value to consider button as pressed.
> + */
> +struct button_adc_priv {
> +struct udevice *adc;
> +int channel;
> +int min;
> +int max;
> +};
> +
> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> +{
> +struct button_adc_priv *priv = dev_get_priv(dev);
> +unsigned int val;
> +int ret;
> +
> +ret = adc_start_channel(priv->adc, priv->channel);
> +if (ret)
> +return ret;
> +
> +ret = adc_channel_data(priv->adc, priv->channel, );
> +if (ret)
> +return ret;
> +
> +if (ret == 0)
> +return (val >= priv->min && val < priv->max) ?
> +BUTTON_ON : BUTTON_OFF;
> +
> +return ret;
> +}
> +
> +static int button_adc_probe(struct udevice *dev)
> +{
> +struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> +struct button_adc_priv *priv = dev_get_priv(dev);
> +struct ofnode_phandle_args args;
> +u32 treshold, up_treshold, t;
> +unsigned int mask;
> +ofnode node;
> +int ret, vdd;
> +
> +/* Ignore the top-level button node */
> +if (!uc_plat->label)
> +return 0;
> +
> +ret = dev_read_phandle_with_args(dev->parent, "io-channels",
> + "#io-channel-cells", 0, 0, );
> +if (ret)
> +return ret;
> +
> +ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node,
> >adc);
> +if (ret)
> +return ret;
> +
> +ret = ofnode_read_u32(dev_ofnode(dev->parent),
> +  "keyup-threshold-microvolt", _treshold);
> +if (ret)
> +return ret;
> +
> +ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
> +  );
> +if (ret)
> +return ret;
> +
> +dev_for_each_subnode(node, dev->parent) {
> +ret = ofnode_read_u32(dev_ofnode(dev),
> +  "press-threshold-microvolt", );
> +

Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-02-01 Thread Simon Glass
Hi,

On Tue, 26 Jan 2021 at 06:03, Heinrich Schuchardt  wrote:
>
> On 26.01.21 12:25, Marek Szyprowski wrote:
> > Hi Heinrich,
> >
> > On 26.01.2021 12:10, Heinrich Schuchardt wrote:
> >> On 1/26/21 10:50 AM, Marek Szyprowski wrote:
> >>> Add a simple Analog to Digital Converter device based button driver.
> >>> This
> >>> driver binds to the 'adc-keys' device tree node.
> >>>
> >>> Signed-off-by: Marek Szyprowski 
> >>> ---
> >>>   drivers/button/Kconfig  |   8 ++
> >>>   drivers/button/Makefile |   1 +
> >>>   drivers/button/button-adc.c | 156 
> >>>   3 files changed, 165 insertions(+)
> >>>   create mode 100644 drivers/button/button-adc.c
> >>>
> >>> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> >>> index 6b3ec7e55d..6db3c5e93a 100644
> >>> --- a/drivers/button/Kconfig
> >>> +++ b/drivers/button/Kconfig
> >>> @@ -9,6 +9,14 @@ config BUTTON
> >>> can provide access to board-specific buttons. Use of the
> >>> device tree
> >>> for configuration is encouraged.
> >>>
> >>> +config BUTTON_ADC
> >>> +bool "Button adc"
> >>> +depends on BUTTON
> >>> +help
> >>> +  Enable support for buttons which are connected to Analog to
> >>> Digital
> >>> +  Converter device. The ADC driver must use driver model.
> >>> Buttons are
> >>> +  configured using the device tree.
> >>> +
> >>>   config BUTTON_GPIO
> >>>   bool "Button gpio"
> >>>   depends on BUTTON
> >>> diff --git a/drivers/button/Makefile b/drivers/button/Makefile
> >>> index fcc10ebe8d..bbd18af149 100644
> >>> --- a/drivers/button/Makefile
> >>> +++ b/drivers/button/Makefile
> >>> @@ -3,4 +3,5 @@
> >>>   # Copyright (C) 2020 Philippe Reynes 
> >>>
> >>>   obj-$(CONFIG_BUTTON) += button-uclass.o
> >>> +obj-$(CONFIG_BUTTON_ADC) += button-adc.o
> >>>   obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
> >>> diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c
> >>> new file mode 100644
> >>> index 00..1901d59a0e
> >>> --- /dev/null
> >>> +++ b/drivers/button/button-adc.c
> >>> @@ -0,0 +1,156 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Copyright (C) 2021 Samsung Electronics Co., Ltd.
> >>> + *http://www.samsung.com
> >>> + * Author: Marek Szyprowski 
> >>> + */
> >>> +
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +
> >>> +/**
> >>> + * struct button_adc_priv - private data for button-adc driver.
> >>> + *
> >>> + * @adc: Analog to Digital Converter device to which button is
> >>> connected.
> >>> + * @channel: channel of the ADC device to probe the button state.
> >>> + * @min: minimal raw ADC sample value to consider button as pressed.
> >>> + * @max: maximal raw ADC sample value to consider button as pressed.
> >>> + */
> >>> +struct button_adc_priv {
> >>> +struct udevice *adc;
> >>> +int channel;
> >>> +int min;
> >>> +int max;
> >>> +};
> >>> +
> >>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> >>> +{
> >>> +struct button_adc_priv *priv = dev_get_priv(dev);
> >>> +unsigned int val;
> >>> +int ret;
> >>> +
> >>> +ret = adc_start_channel(priv->adc, priv->channel);
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +ret = adc_channel_data(priv->adc, priv->channel, );
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +if (ret == 0)
> >>> +return (val >= priv->min && val < priv->max) ?
> >>> +BUTTON_ON : BUTTON_OFF;
> >>> +
> >>> +return ret;
> >>> +}
> >>> +
> >>> +static int button_adc_probe(struct udevice *dev)
> >>> +{
> >>> +struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> >>> +struct button_adc_priv *priv = dev_get_priv(dev);
> >>> +struct ofnode_phandle_args args;
> >>> +u32 treshold, up_treshold, t;
> >>> +unsigned int mask;
> >>> +ofnode node;
> >>> +int ret, vdd;
> >>> +
> >>> +/* Ignore the top-level button node */
> >>> +if (!uc_plat->label)
> >>> +return 0;
> >>> +
> >>> +ret = dev_read_phandle_with_args(dev->parent, "io-channels",
> >>> + "#io-channel-cells", 0, 0, );
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node,
> >>> >adc);
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +ret = ofnode_read_u32(dev_ofnode(dev->parent),
> >>> +  "keyup-threshold-microvolt", _treshold);
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
> >>> +  );
> >>> +if (ret)
> >>> +return ret;
> >>> +
> >>> +dev_for_each_subnode(node, dev->parent) {
> >>> +ret = ofnode_read_u32(dev_ofnode(dev),
> >>> +  "press-threshold-microvolt", );
> >>> +if (ret)
> >>> +return ret;

Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-01-26 Thread Heinrich Schuchardt
On 26.01.21 12:25, Marek Szyprowski wrote:
> Hi Heinrich,
>
> On 26.01.2021 12:10, Heinrich Schuchardt wrote:
>> On 1/26/21 10:50 AM, Marek Szyprowski wrote:
>>> Add a simple Analog to Digital Converter device based button driver.
>>> This
>>> driver binds to the 'adc-keys' device tree node.
>>>
>>> Signed-off-by: Marek Szyprowski 
>>> ---
>>>   drivers/button/Kconfig  |   8 ++
>>>   drivers/button/Makefile |   1 +
>>>   drivers/button/button-adc.c | 156 
>>>   3 files changed, 165 insertions(+)
>>>   create mode 100644 drivers/button/button-adc.c
>>>
>>> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
>>> index 6b3ec7e55d..6db3c5e93a 100644
>>> --- a/drivers/button/Kconfig
>>> +++ b/drivers/button/Kconfig
>>> @@ -9,6 +9,14 @@ config BUTTON
>>>     can provide access to board-specific buttons. Use of the
>>> device tree
>>>     for configuration is encouraged.
>>>
>>> +config BUTTON_ADC
>>> +    bool "Button adc"
>>> +    depends on BUTTON
>>> +    help
>>> +  Enable support for buttons which are connected to Analog to
>>> Digital
>>> +  Converter device. The ADC driver must use driver model.
>>> Buttons are
>>> +  configured using the device tree.
>>> +
>>>   config BUTTON_GPIO
>>>   bool "Button gpio"
>>>   depends on BUTTON
>>> diff --git a/drivers/button/Makefile b/drivers/button/Makefile
>>> index fcc10ebe8d..bbd18af149 100644
>>> --- a/drivers/button/Makefile
>>> +++ b/drivers/button/Makefile
>>> @@ -3,4 +3,5 @@
>>>   # Copyright (C) 2020 Philippe Reynes 
>>>
>>>   obj-$(CONFIG_BUTTON) += button-uclass.o
>>> +obj-$(CONFIG_BUTTON_ADC) += button-adc.o
>>>   obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
>>> diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c
>>> new file mode 100644
>>> index 00..1901d59a0e
>>> --- /dev/null
>>> +++ b/drivers/button/button-adc.c
>>> @@ -0,0 +1,156 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2021 Samsung Electronics Co., Ltd.
>>> + *    http://www.samsung.com
>>> + * Author: Marek Szyprowski 
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +/**
>>> + * struct button_adc_priv - private data for button-adc driver.
>>> + *
>>> + * @adc: Analog to Digital Converter device to which button is
>>> connected.
>>> + * @channel: channel of the ADC device to probe the button state.
>>> + * @min: minimal raw ADC sample value to consider button as pressed.
>>> + * @max: maximal raw ADC sample value to consider button as pressed.
>>> + */
>>> +struct button_adc_priv {
>>> +    struct udevice *adc;
>>> +    int channel;
>>> +    int min;
>>> +    int max;
>>> +};
>>> +
>>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
>>> +{
>>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>>> +    unsigned int val;
>>> +    int ret;
>>> +
>>> +    ret = adc_start_channel(priv->adc, priv->channel);
>>> +    if (ret)
>>> +    return ret;
>>> +
>>> +    ret = adc_channel_data(priv->adc, priv->channel, );
>>> +    if (ret)
>>> +    return ret;
>>> +
>>> +    if (ret == 0)
>>> +    return (val >= priv->min && val < priv->max) ?
>>> +    BUTTON_ON : BUTTON_OFF;
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int button_adc_probe(struct udevice *dev)
>>> +{
>>> +    struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>>> +    struct ofnode_phandle_args args;
>>> +    u32 treshold, up_treshold, t;
>>> +    unsigned int mask;
>>> +    ofnode node;
>>> +    int ret, vdd;
>>> +
>>> +    /* Ignore the top-level button node */
>>> +    if (!uc_plat->label)
>>> +    return 0;
>>> +
>>> +    ret = dev_read_phandle_with_args(dev->parent, "io-channels",
>>> + "#io-channel-cells", 0, 0, );
>>> +    if (ret)
>>> +    return ret;
>>> +
>>> +    ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node,
>>> >adc);
>>> +    if (ret)
>>> +    return ret;
>>> +
>>> +    ret = ofnode_read_u32(dev_ofnode(dev->parent),
>>> +  "keyup-threshold-microvolt", _treshold);
>>> +    if (ret)
>>> +    return ret;
>>> +
>>> +    ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
>>> +  );
>>> +    if (ret)
>>> +    return ret;
>>> +
>>> +    dev_for_each_subnode(node, dev->parent) {
>>> +    ret = ofnode_read_u32(dev_ofnode(dev),
>>> +  "press-threshold-microvolt", );
>>> +    if (ret)
>>> +    return ret;
>>> +
>>> +    if (t > treshold)
>>> +    up_treshold = t;
>>
>> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
>> that one virtual key is created per sub-node.
>>
>> If I read your code correctly, this is not what you are implementing.
>> Instead you only define a single key per adc-keys node.
>>
>> Why are your deviating 

Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-01-26 Thread Marek Szyprowski
Hi Heinrich,

On 26.01.2021 12:10, Heinrich Schuchardt wrote:
> On 1/26/21 10:50 AM, Marek Szyprowski wrote:
>> Add a simple Analog to Digital Converter device based button driver. 
>> This
>> driver binds to the 'adc-keys' device tree node.
>>
>> Signed-off-by: Marek Szyprowski 
>> ---
>>   drivers/button/Kconfig  |   8 ++
>>   drivers/button/Makefile |   1 +
>>   drivers/button/button-adc.c | 156 
>>   3 files changed, 165 insertions(+)
>>   create mode 100644 drivers/button/button-adc.c
>>
>> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
>> index 6b3ec7e55d..6db3c5e93a 100644
>> --- a/drivers/button/Kconfig
>> +++ b/drivers/button/Kconfig
>> @@ -9,6 +9,14 @@ config BUTTON
>>     can provide access to board-specific buttons. Use of the 
>> device tree
>>     for configuration is encouraged.
>>
>> +config BUTTON_ADC
>> +    bool "Button adc"
>> +    depends on BUTTON
>> +    help
>> +  Enable support for buttons which are connected to Analog to 
>> Digital
>> +  Converter device. The ADC driver must use driver model. 
>> Buttons are
>> +  configured using the device tree.
>> +
>>   config BUTTON_GPIO
>>   bool "Button gpio"
>>   depends on BUTTON
>> diff --git a/drivers/button/Makefile b/drivers/button/Makefile
>> index fcc10ebe8d..bbd18af149 100644
>> --- a/drivers/button/Makefile
>> +++ b/drivers/button/Makefile
>> @@ -3,4 +3,5 @@
>>   # Copyright (C) 2020 Philippe Reynes 
>>
>>   obj-$(CONFIG_BUTTON) += button-uclass.o
>> +obj-$(CONFIG_BUTTON_ADC) += button-adc.o
>>   obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
>> diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c
>> new file mode 100644
>> index 00..1901d59a0e
>> --- /dev/null
>> +++ b/drivers/button/button-adc.c
>> @@ -0,0 +1,156 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2021 Samsung Electronics Co., Ltd.
>> + *    http://www.samsung.com
>> + * Author: Marek Szyprowski 
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/**
>> + * struct button_adc_priv - private data for button-adc driver.
>> + *
>> + * @adc: Analog to Digital Converter device to which button is 
>> connected.
>> + * @channel: channel of the ADC device to probe the button state.
>> + * @min: minimal raw ADC sample value to consider button as pressed.
>> + * @max: maximal raw ADC sample value to consider button as pressed.
>> + */
>> +struct button_adc_priv {
>> +    struct udevice *adc;
>> +    int channel;
>> +    int min;
>> +    int max;
>> +};
>> +
>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
>> +{
>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>> +    unsigned int val;
>> +    int ret;
>> +
>> +    ret = adc_start_channel(priv->adc, priv->channel);
>> +    if (ret)
>> +    return ret;
>> +
>> +    ret = adc_channel_data(priv->adc, priv->channel, );
>> +    if (ret)
>> +    return ret;
>> +
>> +    if (ret == 0)
>> +    return (val >= priv->min && val < priv->max) ?
>> +    BUTTON_ON : BUTTON_OFF;
>> +
>> +    return ret;
>> +}
>> +
>> +static int button_adc_probe(struct udevice *dev)
>> +{
>> +    struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>> +    struct ofnode_phandle_args args;
>> +    u32 treshold, up_treshold, t;
>> +    unsigned int mask;
>> +    ofnode node;
>> +    int ret, vdd;
>> +
>> +    /* Ignore the top-level button node */
>> +    if (!uc_plat->label)
>> +    return 0;
>> +
>> +    ret = dev_read_phandle_with_args(dev->parent, "io-channels",
>> + "#io-channel-cells", 0, 0, );
>> +    if (ret)
>> +    return ret;
>> +
>> +    ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node, 
>> >adc);
>> +    if (ret)
>> +    return ret;
>> +
>> +    ret = ofnode_read_u32(dev_ofnode(dev->parent),
>> +  "keyup-threshold-microvolt", _treshold);
>> +    if (ret)
>> +    return ret;
>> +
>> +    ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
>> +  );
>> +    if (ret)
>> +    return ret;
>> +
>> +    dev_for_each_subnode(node, dev->parent) {
>> +    ret = ofnode_read_u32(dev_ofnode(dev),
>> +  "press-threshold-microvolt", );
>> +    if (ret)
>> +    return ret;
>> +
>> +    if (t > treshold)
>> +    up_treshold = t;
>
> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
> that one virtual key is created per sub-node.
>
> If I read your code correctly, this is not what you are implementing.
> Instead you only define a single key per adc-keys node.
>
> Why are your deviating from the bindings document?

No I don't. button_adc_bind() binds to the root node with 'adc-keys' 
compatible, while the dev_for_each_subnode() loop instantiates driver 
for each subnode, so the 

Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-01-26 Thread Heinrich Schuchardt

On 1/26/21 10:50 AM, Marek Szyprowski wrote:

Add a simple Analog to Digital Converter device based button driver. This
driver binds to the 'adc-keys' device tree node.

Signed-off-by: Marek Szyprowski 
---
  drivers/button/Kconfig  |   8 ++
  drivers/button/Makefile |   1 +
  drivers/button/button-adc.c | 156 
  3 files changed, 165 insertions(+)
  create mode 100644 drivers/button/button-adc.c

diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
index 6b3ec7e55d..6db3c5e93a 100644
--- a/drivers/button/Kconfig
+++ b/drivers/button/Kconfig
@@ -9,6 +9,14 @@ config BUTTON
  can provide access to board-specific buttons. Use of the device tree
  for configuration is encouraged.

+config BUTTON_ADC
+   bool "Button adc"
+   depends on BUTTON
+   help
+ Enable support for buttons which are connected to Analog to Digital
+ Converter device. The ADC driver must use driver model. Buttons are
+ configured using the device tree.
+
  config BUTTON_GPIO
bool "Button gpio"
depends on BUTTON
diff --git a/drivers/button/Makefile b/drivers/button/Makefile
index fcc10ebe8d..bbd18af149 100644
--- a/drivers/button/Makefile
+++ b/drivers/button/Makefile
@@ -3,4 +3,5 @@
  # Copyright (C) 2020 Philippe Reynes 

  obj-$(CONFIG_BUTTON) += button-uclass.o
+obj-$(CONFIG_BUTTON_ADC) += button-adc.o
  obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c
new file mode 100644
index 00..1901d59a0e
--- /dev/null
+++ b/drivers/button/button-adc.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ * Author: Marek Szyprowski 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * struct button_adc_priv - private data for button-adc driver.
+ *
+ * @adc: Analog to Digital Converter device to which button is connected.
+ * @channel: channel of the ADC device to probe the button state.
+ * @min: minimal raw ADC sample value to consider button as pressed.
+ * @max: maximal raw ADC sample value to consider button as pressed.
+ */
+struct button_adc_priv {
+   struct udevice *adc;
+   int channel;
+   int min;
+   int max;
+};
+
+static enum button_state_t button_adc_get_state(struct udevice *dev)
+{
+   struct button_adc_priv *priv = dev_get_priv(dev);
+   unsigned int val;
+   int ret;
+
+   ret = adc_start_channel(priv->adc, priv->channel);
+   if (ret)
+   return ret;
+
+   ret = adc_channel_data(priv->adc, priv->channel, );
+   if (ret)
+   return ret;
+
+   if (ret == 0)
+   return (val >= priv->min && val < priv->max) ?
+   BUTTON_ON : BUTTON_OFF;
+
+   return ret;
+}
+
+static int button_adc_probe(struct udevice *dev)
+{
+   struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+   struct button_adc_priv *priv = dev_get_priv(dev);
+   struct ofnode_phandle_args args;
+   u32 treshold, up_treshold, t;
+   unsigned int mask;
+   ofnode node;
+   int ret, vdd;
+
+   /* Ignore the top-level button node */
+   if (!uc_plat->label)
+   return 0;
+
+   ret = dev_read_phandle_with_args(dev->parent, "io-channels",
+"#io-channel-cells", 0, 0, );
+   if (ret)
+   return ret;
+
+   ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node, >adc);
+   if (ret)
+   return ret;
+
+   ret = ofnode_read_u32(dev_ofnode(dev->parent),
+ "keyup-threshold-microvolt", _treshold);
+   if (ret)
+   return ret;
+
+   ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
+ );
+   if (ret)
+   return ret;
+
+   dev_for_each_subnode(node, dev->parent) {
+   ret = ofnode_read_u32(dev_ofnode(dev),
+ "press-threshold-microvolt", );
+   if (ret)
+   return ret;
+
+   if (t > treshold)
+   up_treshold = t;


Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
that one virtual key is created per sub-node.

If I read your code correctly, this is not what you are implementing.
Instead you only define a single key per adc-keys node.

Why are your deviating from the bindings document?

Best regards

Heinrich


+   }
+
+   ret = adc_vdd_value(priv->adc, );
+   if (ret)
+   return ret;
+
+   ret = adc_data_mask(priv->adc, );
+   if (ret)
+   return ret;
+
+   priv->channel = args.args[0];
+   priv->min = mask * (treshold / 1000) / (vdd / 1000);
+   priv->max = mask * (up_treshold / 1000) / (vdd / 1000);
+
+   

[PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver

2021-01-26 Thread Marek Szyprowski
Add a simple Analog to Digital Converter device based button driver. This
driver binds to the 'adc-keys' device tree node.

Signed-off-by: Marek Szyprowski 
---
 drivers/button/Kconfig  |   8 ++
 drivers/button/Makefile |   1 +
 drivers/button/button-adc.c | 156 
 3 files changed, 165 insertions(+)
 create mode 100644 drivers/button/button-adc.c

diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
index 6b3ec7e55d..6db3c5e93a 100644
--- a/drivers/button/Kconfig
+++ b/drivers/button/Kconfig
@@ -9,6 +9,14 @@ config BUTTON
  can provide access to board-specific buttons. Use of the device tree
  for configuration is encouraged.
 
+config BUTTON_ADC
+   bool "Button adc"
+   depends on BUTTON
+   help
+ Enable support for buttons which are connected to Analog to Digital
+ Converter device. The ADC driver must use driver model. Buttons are
+ configured using the device tree.
+
 config BUTTON_GPIO
bool "Button gpio"
depends on BUTTON
diff --git a/drivers/button/Makefile b/drivers/button/Makefile
index fcc10ebe8d..bbd18af149 100644
--- a/drivers/button/Makefile
+++ b/drivers/button/Makefile
@@ -3,4 +3,5 @@
 # Copyright (C) 2020 Philippe Reynes 
 
 obj-$(CONFIG_BUTTON) += button-uclass.o
+obj-$(CONFIG_BUTTON_ADC) += button-adc.o
 obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c
new file mode 100644
index 00..1901d59a0e
--- /dev/null
+++ b/drivers/button/button-adc.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ * Author: Marek Szyprowski 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * struct button_adc_priv - private data for button-adc driver.
+ *
+ * @adc: Analog to Digital Converter device to which button is connected.
+ * @channel: channel of the ADC device to probe the button state.
+ * @min: minimal raw ADC sample value to consider button as pressed.
+ * @max: maximal raw ADC sample value to consider button as pressed.
+ */
+struct button_adc_priv {
+   struct udevice *adc;
+   int channel;
+   int min;
+   int max;
+};
+
+static enum button_state_t button_adc_get_state(struct udevice *dev)
+{
+   struct button_adc_priv *priv = dev_get_priv(dev);
+   unsigned int val;
+   int ret;
+
+   ret = adc_start_channel(priv->adc, priv->channel);
+   if (ret)
+   return ret;
+
+   ret = adc_channel_data(priv->adc, priv->channel, );
+   if (ret)
+   return ret;
+
+   if (ret == 0)
+   return (val >= priv->min && val < priv->max) ?
+   BUTTON_ON : BUTTON_OFF;
+
+   return ret;
+}
+
+static int button_adc_probe(struct udevice *dev)
+{
+   struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+   struct button_adc_priv *priv = dev_get_priv(dev);
+   struct ofnode_phandle_args args;
+   u32 treshold, up_treshold, t;
+   unsigned int mask;
+   ofnode node;
+   int ret, vdd;
+
+   /* Ignore the top-level button node */
+   if (!uc_plat->label)
+   return 0;
+
+   ret = dev_read_phandle_with_args(dev->parent, "io-channels",
+"#io-channel-cells", 0, 0, );
+   if (ret)
+   return ret;
+
+   ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node, >adc);
+   if (ret)
+   return ret;
+
+   ret = ofnode_read_u32(dev_ofnode(dev->parent),
+ "keyup-threshold-microvolt", _treshold);
+   if (ret)
+   return ret;
+
+   ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
+ );
+   if (ret)
+   return ret;
+
+   dev_for_each_subnode(node, dev->parent) {
+   ret = ofnode_read_u32(dev_ofnode(dev),
+ "press-threshold-microvolt", );
+   if (ret)
+   return ret;
+
+   if (t > treshold)
+   up_treshold = t;
+   }
+
+   ret = adc_vdd_value(priv->adc, );
+   if (ret)
+   return ret;
+
+   ret = adc_data_mask(priv->adc, );
+   if (ret)
+   return ret;
+
+   priv->channel = args.args[0];
+   priv->min = mask * (treshold / 1000) / (vdd / 1000);
+   priv->max = mask * (up_treshold / 1000) / (vdd / 1000);
+
+   return ret;
+}
+
+static int button_adc_bind(struct udevice *parent)
+{
+   struct udevice *dev;
+   ofnode node;
+   int ret;
+
+   dev_for_each_subnode(node, parent) {
+   struct button_uc_plat *uc_plat;
+   const char *label;
+
+   label = ofnode_read_string(node, "label");
+   if (!label) {
+