subject:"\[PATCH 2\/6\] mfd\: Add ST Multi\-Function eXpander core driver"

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

2018-02-22 Thread Amelie DELAUNAY



On 02/22/2018 02:44 PM, Linus Walleij wrote:
> On Thu, Feb 8, 2018 at 3:27 PM, Amelie Delaunay  
> wrote:
> 
> Thanks for working on this complex expander driver.
> It is a bit daunting. Sorry if there are lots of comments and
> considerations, but it reflects the complexity of the hardware.
> 
>> +enum mfx_block {
>> +   MFX_BLOCK_GPIO  = BIT(0),
>> +   MFX_BLOCK_TS= BIT(1),
>> +   MFX_BLOCK_IDD   = BIT(2),
>> +   MFX_BLOCK_ALTGPIO   = BIT(3),
>> +};
> 
> This looks suspiciously similar to this:
> 
> enum stmpe_block {
>  STMPE_BLOCK_GPIO= 1 << 0,
>  STMPE_BLOCK_KEYPAD  = 1 << 1,
>  STMPE_BLOCK_TOUCHSCREEN = 1 << 2,
>  STMPE_BLOCK_ADC = 1 << 3,
>  STMPE_BLOCK_PWM = 1 << 4,
>  STMPE_BLOCK_ROTATOR = 1 << 5,
> };
> 
> Apparently the same hardware designers are involved.
> 

Or the firmware developers were heavely inspired by STMPE!

>> +int mfx_reg_write(struct mfx *mfx, u8 reg, u8 data);
>> +int mfx_reg_read(struct mfx *mfx, u8 reg);
>> +int mfx_block_read(struct mfx *mfx, u8 reg, u8 length, u8 *values);
>> +int mfx_block_write(struct mfx *mfx, u8 reg, u8 length, const u8 *values);
>> +int mfx_set_bits(struct mfx *mfx, u8 reg, u8 mask, u8 val);
> 
> Do you need this? Can't you just use regmap and pass
> around a struct regmap *map to access registers?
> 
> You don't necessarily need to use the default I2C regmap
> (like, e.g. drivers/mfd/stw481x.c) but even if a more
> complex access pattern is used to read/write registers
> I am pretty sure you can use regmap for it.
> 

Yes, Lee has also raised this point.

>> +int mfx_enable(struct mfx *mfx, unsigned int blocks);
>> +int mfx_disable(struct mfx *mfx, unsigned int blocks);
> 
> This is similar to
> extern int stmpe_enable(struct stmpe *stmpe, unsigned int blocks);
> extern int stmpe_disable(struct stmpe *stmpe, unsigned int blocks);
> 
> So again I am suspicious about duplication of driver code.
> 
> It even looks a bit like this driver started as a copy of
> the STMPE driver, which is not a good sign. It might be
> that it was copied from there because the hardware is
> actually very similar.
> 

HW is different, FW looks like similar.

>> +/* General */
>> +#define MFX_REG_CHIP_ID0x00 /* R */
>> +#define MFX_REG_FW_VERSION_MSB 0x01 /* R */
>> +#define MFX_REG_FW_VERSION_LSB 0x02 /* R */
>> +#define MFX_REG_SYS_CTRL   0x40 /* RW */
> 
> The STMPE driver defines enumerated registers in
> include/linux/mfd/stmpe.h
> then assign each variant using the model specifics in
> drivers/mfd/stmpe.h
> 
> This doesn't seem super much different.
> 
> Even if the old STMPE driver may be a bad fit, is is better
> to improve that (e.g. migrate it to use regmap and rewrite the
> stmpe-gpio.c driver to use pin control) and use also for this
> driver, or write a new driver from scratch like this?
> 
> I'm not so sure.
> 
> I do know that developers not always like to take out old
> hardware and old development boards and start hacking
> away before they can get some nice new hardware going
> but I am worried that this may be one of those cases where
> a serious cleanup of the aging STMPE driver may be a
> first necessary step.
> 
Just looking at [1], we see that the STMPE remaining active are the 
STMPE1600 and STMPE1801. All the others are obsolete.

[1] 
http://www.st.com/en/interfaces-and-transceivers/i-o-expanders.html?querycriteria=productId=SC1027

>> +/* IRQ output management */
>> +#define MFX_REG_IRQ_OUT_PIN0x41 /* RW */
>> +#define MFX_REG_IRQ_SRC_EN 0x42 /* RW */
>> +#define MFX_REG_IRQ_PENDING0x08 /* R */
>> +#define MFX_REG_IRQ_ACK0x44 /* RW */
> 
> Very similar to STMPE it seems.
> 

IRQ_OUT_PIN is different (edge not supported, open drain or push pull 
output) and IRQ_ACK new.

>> +/* MFX_REG_SYS_CTRL bitfields */
>> +#define MFX_REG_SYS_CTRL_GPIO_EN   BIT(0)
>> +#define MFX_REG_SYS_CTRL_TS_EN BIT(1)
>> +#define MFX_REG_SYS_CTRL_IDD_ENBIT(2)
>> +#define MFX_REG_SYS_CTRL_ALTGPIO_ENBIT(3)
> 
> I guess these blocks works the same as with STMPE,
> that you can only use one of them at the time?
> 
> What is altgpio?
> 

ALTGPIO enables the use of GPIO[23:16] only if IDD and/or TS is/are 
disabled. TS and IDD have priority, if IDD and TS are enabled, ALTGPIO 
is forced to 0 by MFX FW.
When IDD is used GPIO[19:16] can be used as GPIO.
When TS is used GPIO[24:20] can be used as GPIO.

>> +/* MFX_REG_IRQ_OUT_PIN bitfields */
>> +#define MFX_REG_IRQ_OUT_PIN_TYPE   BIT(0) /* 0-OD 1-PP */
>> +#define MFX_REG_IRQ_OUT_PIN_POLBIT(1) /* 0-active LOW 
>> 1-active HIGH */
> 
> I have not read the patch yet. But just for notice:
> This output IRQ type needs to be handled as well.
> 
> Check the code in
> drivers/iio/common/st_sensors/st_sensors_trigger.c
> 
> To see how you can detect the

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

2018-02-22 Thread Linus Walleij

On Thu, Feb 8, 2018 at 3:27 PM, Amelie Delaunay  wrote:

Thanks for working on this complex expander driver.
It is a bit daunting. Sorry if there are lots of comments and
considerations, but it reflects the complexity of the hardware.

> +enum mfx_block {
> +   MFX_BLOCK_GPIO  = BIT(0),
> +   MFX_BLOCK_TS= BIT(1),
> +   MFX_BLOCK_IDD   = BIT(2),
> +   MFX_BLOCK_ALTGPIO   = BIT(3),
> +};

This looks suspiciously similar to this:

enum stmpe_block {
STMPE_BLOCK_GPIO= 1 << 0,
STMPE_BLOCK_KEYPAD  = 1 << 1,
STMPE_BLOCK_TOUCHSCREEN = 1 << 2,
STMPE_BLOCK_ADC = 1 << 3,
STMPE_BLOCK_PWM = 1 << 4,
STMPE_BLOCK_ROTATOR = 1 << 5,
};

Apparently the same hardware designers are involved.

> +int mfx_reg_write(struct mfx *mfx, u8 reg, u8 data);
> +int mfx_reg_read(struct mfx *mfx, u8 reg);
> +int mfx_block_read(struct mfx *mfx, u8 reg, u8 length, u8 *values);
> +int mfx_block_write(struct mfx *mfx, u8 reg, u8 length, const u8 *values);
> +int mfx_set_bits(struct mfx *mfx, u8 reg, u8 mask, u8 val);

Do you need this? Can't you just use regmap and pass
around a struct regmap *map to access registers?

You don't necessarily need to use the default I2C regmap
(like, e.g. drivers/mfd/stw481x.c) but even if a more
complex access pattern is used to read/write registers
I am pretty sure you can use regmap for it.

> +int mfx_enable(struct mfx *mfx, unsigned int blocks);
> +int mfx_disable(struct mfx *mfx, unsigned int blocks);

This is similar to
extern int stmpe_enable(struct stmpe *stmpe, unsigned int blocks);
extern int stmpe_disable(struct stmpe *stmpe, unsigned int blocks);

So again I am suspicious about duplication of driver code.

It even looks a bit like this driver started as a copy of
the STMPE driver, which is not a good sign. It might be
that it was copied from there because the hardware is
actually very similar.

> +/* General */
> +#define MFX_REG_CHIP_ID0x00 /* R */
> +#define MFX_REG_FW_VERSION_MSB 0x01 /* R */
> +#define MFX_REG_FW_VERSION_LSB 0x02 /* R */
> +#define MFX_REG_SYS_CTRL   0x40 /* RW */

The STMPE driver defines enumerated registers in
include/linux/mfd/stmpe.h
then assign each variant using the model specifics in
drivers/mfd/stmpe.h

This doesn't seem super much different.

Even if the old STMPE driver may be a bad fit, is is better
to improve that (e.g. migrate it to use regmap and rewrite the
stmpe-gpio.c driver to use pin control) and use also for this
driver, or write a new driver from scratch like this?

I'm not so sure.

I do know that developers not always like to take out old
hardware and old development boards and start hacking
away before they can get some nice new hardware going
but I am worried that this may be one of those cases where
a serious cleanup of the aging STMPE driver may be a
first necessary step.

> +/* IRQ output management */
> +#define MFX_REG_IRQ_OUT_PIN0x41 /* RW */
> +#define MFX_REG_IRQ_SRC_EN 0x42 /* RW */
> +#define MFX_REG_IRQ_PENDING0x08 /* R */
> +#define MFX_REG_IRQ_ACK0x44 /* RW */

Very similar to STMPE it seems.

> +/* MFX_REG_SYS_CTRL bitfields */
> +#define MFX_REG_SYS_CTRL_GPIO_EN   BIT(0)
> +#define MFX_REG_SYS_CTRL_TS_EN BIT(1)
> +#define MFX_REG_SYS_CTRL_IDD_ENBIT(2)
> +#define MFX_REG_SYS_CTRL_ALTGPIO_ENBIT(3)

I guess these blocks works the same as with STMPE,
that you can only use one of them at the time?

What is altgpio?

> +/* MFX_REG_IRQ_OUT_PIN bitfields */
> +#define MFX_REG_IRQ_OUT_PIN_TYPE   BIT(0) /* 0-OD 1-PP */
> +#define MFX_REG_IRQ_OUT_PIN_POLBIT(1) /* 0-active LOW 
> 1-active HIGH */

I have not read the patch yet. But just for notice:
This output IRQ type needs to be handled as well.

Check the code in
drivers/iio/common/st_sensors/st_sensors_trigger.c

To see how you can detect the properties of an IRQ
to set the right polarity, and handling of open drain
IRQ lines.

Yours,
Linus Walleij

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

2018-02-19 Thread Amelie DELAUNAY



On 02/12/2018 01:06 PM, Lee Jones wrote:
> On Thu, 08 Feb 2018, Amelie Delaunay wrote:
> 
>> ST Multi-Function eXpander (MFX) is a slave controller using I2C
>> for communication with the main MCU. Main features are:
>> - 16 fast GPIOs individually configurable in input/output
>> - 8 alternate GPIOs individually configurable in input/output
>> - Main MCU IDD measurement
>> - Resistive touchscreen controller
>>
>> Only GPIO expander (16 fast GPIOs + 8 alternate) feature is
>> supported for the moment.
>>
>> Signed-off-by: Amelie Delaunay 
>> ---
>>   drivers/mfd/Kconfig|  15 ++
>>   drivers/mfd/Makefile   |   1 +
>>   drivers/mfd/st-mfx.c   | 526 
>> +
>>   include/linux/mfd/st-mfx.h | 116 ++
>>   4 files changed, 658 insertions(+)
>>   create mode 100644 drivers/mfd/st-mfx.c
>>   create mode 100644 include/linux/mfd/st-mfx.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 1d20a80..e78e818 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -1823,6 +1823,21 @@ config MFD_STM32_TIMERS
>>for PWM and IIO Timer. This driver allow to share the
>>registers between the others drivers.
>>   
>> +config MFD_ST_MFX
>> +bool "STMicroelectronics MFX"
>> +depends on I2C
>> +depends on OF || COMPILE_TEST
>> +select MFD_CORE
>> +select REGMAP_I2C
>> +help
>> +  Support for the STMicroelectronics Multi-Function eXpander.
> 
> Is that really what this device is called?
Yes, all STMicroelectronics Evaluation boards and Discovery kits (using 
an expansion device) user manuals refer to "MFX (Multi Function eXpander)".
> 
> Is there a datasheet?
> 
>> +  This driver provides common support for accessing the device,
>> +  additional drivers must be enabled in order to use the functionality
>> +  of the device. Currently available sub drivers are:
>> +
>> +GPIO: mfx-gpio
> 
> This driver would be easier to review if I could picture the device as
> a whole.  What other functionality does it have?  What is it that
> makes this an MFD?
>
MFX is a slave controller based on the STM32L152CCT6 MCU.
MFX firmware integrates 3 features:
- main MCU IDD measurement: this allows measurement of the IDD current 
consumed by the main MCU, especially for static measurement in low power 
mode.
- resistive touchscreen controller, using ADC inputs of STM32L152CCT6.
- GPIO expander: 16 programmable input/output + 8 alternate GPIO 
(Touchscreen controller uses the four first gpios of this bank when 
enabled, IDD used the 4 last ones when enabled).

IDD measurement driver and touchscreen driver have not been developed 
yet because I have no hardware to test it. But anyway, I will send a V2 
with gpio driver only.

>>   menu "Multimedia Capabilities Port drivers"
>>  depends on ARCH_SA1100
>>   
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index d9474ad..1379a18 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -230,3 +230,4 @@ obj-$(CONFIG_MFD_STM32_LPTIMER)  += stm32-lptimer.o
>>   obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
>>   obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
>>   obj-$(CONFIG_MFD_SC27XX_PMIC)  += sprd-sc27xx-spi.o
>> +obj-$(CONFIG_MFD_ST_MFX)+= st-mfx.o
>> diff --git a/drivers/mfd/st-mfx.c b/drivers/mfd/st-mfx.c
>> new file mode 100644
>> index 000..5bee5d3
>> --- /dev/null
>> +++ b/drivers/mfd/st-mfx.c
>> @@ -0,0 +1,526 @@
>> +/*
>> + * STMicroelectronics Multi-Function eXpander (ST-MFX) Core Driver
>> + *
>> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
>> + * Author(s): Amelie Delaunay  for 
>> STMicroelectronics.
> 
> You don't need to put "for STMicroelectronics".  This was something we
> made up when submitting from a different (!st.com) email address.
> 
OK, I will fix it in gpio driver for V2.
>> + * License terms: GPL V2.0.
>> + *
>> + * st-mfx Core Driver is free software; you can redistribute it and/or 
>> modify it
>> + * under the terms of the GNU General Public License version 2 as published 
>> by
>> + * the Free Software Foundation.
>> + *
>> + * st-mfx Core Driver is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of 
>> MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for 
>> more
>> + * details.
>> + *
>> + * You should have received a copy of the GNU General Public License along 
>> with
>> + * st-mfx Core Driver. If not, see .
> 
> You should be able to use the short version of the licensing
> agreement.  Also, please grep for "SPDX".
> 
OK.
>> + */
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/**
>> + * struct mfx_priv - MFX MFD private structure
>> + * @dev: device, mostly for logs
>> + * @regmap: register map

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

2018-02-19 Thread Amelie DELAUNAY

Thanks Philippe,

I will take this into account for V2.

Regards,
Amelie

On 02/12/2018 03:15 PM, Philippe Ombredanne wrote:
> Amelie,
> 
> On Mon, Feb 12, 2018 at 1:06 PM, Lee Jones  wrote:
>> On Thu, 08 Feb 2018, Amelie Delaunay wrote:
>>
>>> ST Multi-Function eXpander (MFX) is a slave controller using I2C
>>> for communication with the main MCU. Main features are:
>>> - 16 fast GPIOs individually configurable in input/output
>>> - 8 alternate GPIOs individually configurable in input/output
>>> - Main MCU IDD measurement
>>> - Resistive touchscreen controller
>>>
>>> Only GPIO expander (16 fast GPIOs + 8 alternate) feature is
>>> supported for the moment.
>>>
>>> Signed-off-by: Amelie Delaunay 
> 
> 
> 
>>> --- /dev/null
>>> +++ b/drivers/mfd/st-mfx.c
>>> @@ -0,0 +1,526 @@
>>> +/*
>>> + * STMicroelectronics Multi-Function eXpander (ST-MFX) Core Driver
>>> + *
>>> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
>>> + * Author(s): Amelie Delaunay  for 
>>> STMicroelectronics.
>>
>> You don't need to put "for STMicroelectronics".  This was something we
>> made up when submitting from a different (!st.com) email address.
>>
>>> + * License terms: GPL V2.0.
>>> + *
>>> + * st-mfx Core Driver is free software; you can redistribute it and/or 
>>> modify it
>>> + * under the terms of the GNU General Public License version 2 as 
>>> published by
>>> + * the Free Software Foundation.
>>> + *
>>> + * st-mfx Core Driver is distributed in the hope that it will be useful, 
>>> but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of 
>>> MERCHANTABILITY or
>>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 
>>> for more
>>> + * details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along 
>>> with
>>> + * st-mfx Core Driver. If not, see .
>>
>> You should be able to use the short version of the licensing
>> agreement.  Also, please grep for "SPDX".
> 
> You can check the doc for the (fairly new) way to remove legalese
> boilerplate at Documentation/process/license-rules.rst or [1]
> It helps keep the focus on the code and less on licensing!
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/license-rules.rst
>

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

2018-02-12 Thread Philippe Ombredanne

Amelie,

On Mon, Feb 12, 2018 at 1:06 PM, Lee Jones  wrote:
> On Thu, 08 Feb 2018, Amelie Delaunay wrote:
>
>> ST Multi-Function eXpander (MFX) is a slave controller using I2C
>> for communication with the main MCU. Main features are:
>> - 16 fast GPIOs individually configurable in input/output
>> - 8 alternate GPIOs individually configurable in input/output
>> - Main MCU IDD measurement
>> - Resistive touchscreen controller
>>
>> Only GPIO expander (16 fast GPIOs + 8 alternate) feature is
>> supported for the moment.
>>
>> Signed-off-by: Amelie Delaunay 



>> --- /dev/null
>> +++ b/drivers/mfd/st-mfx.c
>> @@ -0,0 +1,526 @@
>> +/*
>> + * STMicroelectronics Multi-Function eXpander (ST-MFX) Core Driver
>> + *
>> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
>> + * Author(s): Amelie Delaunay  for 
>> STMicroelectronics.
>
> You don't need to put "for STMicroelectronics".  This was something we
> made up when submitting from a different (!st.com) email address.
>
>> + * License terms: GPL V2.0.
>> + *
>> + * st-mfx Core Driver is free software; you can redistribute it and/or 
>> modify it
>> + * under the terms of the GNU General Public License version 2 as published 
>> by
>> + * the Free Software Foundation.
>> + *
>> + * st-mfx Core Driver is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of 
>> MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for 
>> more
>> + * details.
>> + *
>> + * You should have received a copy of the GNU General Public License along 
>> with
>> + * st-mfx Core Driver. If not, see .
>
> You should be able to use the short version of the licensing
> agreement.  Also, please grep for "SPDX".

You can check the doc for the (fairly new) way to remove legalese
boilerplate at Documentation/process/license-rules.rst or [1]
It helps keep the focus on the code and less on licensing!

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/license-rules.rst
-- 
Cordially
Philippe Ombredanne

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

2018-02-12 Thread Lee Jones

On Thu, 08 Feb 2018, Amelie Delaunay wrote:

> ST Multi-Function eXpander (MFX) is a slave controller using I2C
> for communication with the main MCU. Main features are:
> - 16 fast GPIOs individually configurable in input/output
> - 8 alternate GPIOs individually configurable in input/output
> - Main MCU IDD measurement
> - Resistive touchscreen controller
> 
> Only GPIO expander (16 fast GPIOs + 8 alternate) feature is
> supported for the moment.
> 
> Signed-off-by: Amelie Delaunay 
> ---
>  drivers/mfd/Kconfig|  15 ++
>  drivers/mfd/Makefile   |   1 +
>  drivers/mfd/st-mfx.c   | 526 
> +
>  include/linux/mfd/st-mfx.h | 116 ++
>  4 files changed, 658 insertions(+)
>  create mode 100644 drivers/mfd/st-mfx.c
>  create mode 100644 include/linux/mfd/st-mfx.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1d20a80..e78e818 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1823,6 +1823,21 @@ config MFD_STM32_TIMERS
> for PWM and IIO Timer. This driver allow to share the
> registers between the others drivers.
>  
> +config MFD_ST_MFX
> + bool "STMicroelectronics MFX"
> + depends on I2C
> + depends on OF || COMPILE_TEST
> + select MFD_CORE
> + select REGMAP_I2C
> + help
> +   Support for the STMicroelectronics Multi-Function eXpander.

Is that really what this device is called?

Is there a datasheet?

> +   This driver provides common support for accessing the device,
> +   additional drivers must be enabled in order to use the functionality
> +   of the device. Currently available sub drivers are:
> +
> + GPIO: mfx-gpio

This driver would be easier to review if I could picture the device as
a whole.  What other functionality does it have?  What is it that
makes this an MFD?

>  menu "Multimedia Capabilities Port drivers"
>   depends on ARCH_SA1100
>  
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index d9474ad..1379a18 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -230,3 +230,4 @@ obj-$(CONFIG_MFD_STM32_LPTIMER)   += stm32-lptimer.o
>  obj-$(CONFIG_MFD_STM32_TIMERS)   += stm32-timers.o
>  obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
>  obj-$(CONFIG_MFD_SC27XX_PMIC)+= sprd-sc27xx-spi.o
> +obj-$(CONFIG_MFD_ST_MFX) += st-mfx.o
> diff --git a/drivers/mfd/st-mfx.c b/drivers/mfd/st-mfx.c
> new file mode 100644
> index 000..5bee5d3
> --- /dev/null
> +++ b/drivers/mfd/st-mfx.c
> @@ -0,0 +1,526 @@
> +/*
> + * STMicroelectronics Multi-Function eXpander (ST-MFX) Core Driver
> + *
> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
> + * Author(s): Amelie Delaunay  for 
> STMicroelectronics.

You don't need to put "for STMicroelectronics".  This was something we
made up when submitting from a different (!st.com) email address.

> + * License terms: GPL V2.0.
> + *
> + * st-mfx Core Driver is free software; you can redistribute it and/or 
> modify it
> + * under the terms of the GNU General Public License version 2 as published 
> by
> + * the Free Software Foundation.
> + *
> + * st-mfx Core Driver is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of 
> MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for 
> more
> + * details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * st-mfx Core Driver. If not, see .

You should be able to use the short version of the licensing
agreement.  Also, please grep for "SPDX".

> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/**
> + * struct mfx_priv - MFX MFD private structure
> + * @dev: device, mostly for logs
> + * @regmap: register map
> + * @mfx: MFX MFD public structure, to share information with subdrivers
> + * @irq_domain: IRQ domain
> + * @irq: IRQ number for mfx
> + * @irq_lock: IRQ bus lock
> + * @irqen: cache of IRQ_SRC_EN register for bus_lock
> + * @oldirqen: cache of IRQ_SRC_EN register for bus_lock
> + */

/**
 * struct mfx_priv - MFX MFD private structure
 * @dev:   device, mostly for logs
 * @regmap:register map
 * @mfx:   MFX MFD public structure, to share information with 
subdrivers
 * @irq_domain:IRQ domain
 * @irq:   IRQ number for mfx
 * @irq_lock:  IRQ bus lock
 * @irqen: cache of IRQ_SRC_EN register for bus_lock
 * @oldirqen:  cache of IRQ_SRC_EN register for bus_lock
 */

Easier to read?

> +struct mfx_priv {

mfx_ddata

> + struct device *dev;
> + struct regmap *regmap;
> + struct mfx mfx;
> + struct irq_domain *irq_domain;
> + int irq;
> + struct mutex irq_lock; /* IRQ bus lock */
> + u8 irqen;
> + u8 oldirqen;
> +};
> +
> +#define to_mfx_priv

[PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

2018-02-08 Thread Amelie Delaunay

ST Multi-Function eXpander (MFX) is a slave controller using I2C
for communication with the main MCU. Main features are:
- 16 fast GPIOs individually configurable in input/output
- 8 alternate GPIOs individually configurable in input/output
- Main MCU IDD measurement
- Resistive touchscreen controller

Only GPIO expander (16 fast GPIOs + 8 alternate) feature is
supported for the moment.

Signed-off-by: Amelie Delaunay 
---
 drivers/mfd/Kconfig|  15 ++
 drivers/mfd/Makefile   |   1 +
 drivers/mfd/st-mfx.c   | 526 +
 include/linux/mfd/st-mfx.h | 116 ++
 4 files changed, 658 insertions(+)
 create mode 100644 drivers/mfd/st-mfx.c
 create mode 100644 include/linux/mfd/st-mfx.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 1d20a80..e78e818 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1823,6 +1823,21 @@ config MFD_STM32_TIMERS
  for PWM and IIO Timer. This driver allow to share the
  registers between the others drivers.
 
+config MFD_ST_MFX
+   bool "STMicroelectronics MFX"
+   depends on I2C
+   depends on OF || COMPILE_TEST
+   select MFD_CORE
+   select REGMAP_I2C
+   help
+ Support for the STMicroelectronics Multi-Function eXpander.
+
+ This driver provides common support for accessing the device,
+ additional drivers must be enabled in order to use the functionality
+ of the device. Currently available sub drivers are:
+
+   GPIO: mfx-gpio
+
 menu "Multimedia Capabilities Port drivers"
depends on ARCH_SA1100
 
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d9474ad..1379a18 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -230,3 +230,4 @@ obj-$(CONFIG_MFD_STM32_LPTIMER) += stm32-lptimer.o
 obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
 obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
 obj-$(CONFIG_MFD_SC27XX_PMIC)  += sprd-sc27xx-spi.o
+obj-$(CONFIG_MFD_ST_MFX)   += st-mfx.o
diff --git a/drivers/mfd/st-mfx.c b/drivers/mfd/st-mfx.c
new file mode 100644
index 000..5bee5d3
--- /dev/null
+++ b/drivers/mfd/st-mfx.c
@@ -0,0 +1,526 @@
+/*
+ * STMicroelectronics Multi-Function eXpander (ST-MFX) Core Driver
+ *
+ * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
+ * Author(s): Amelie Delaunay  for STMicroelectronics.
+ *
+ * License terms: GPL V2.0.
+ *
+ * st-mfx Core Driver is free software; you can redistribute it and/or modify 
it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * st-mfx Core Driver is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY 
or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for 
more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * st-mfx Core Driver. If not, see .
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * struct mfx_priv - MFX MFD private structure
+ * @dev: device, mostly for logs
+ * @regmap: register map
+ * @mfx: MFX MFD public structure, to share information with subdrivers
+ * @irq_domain: IRQ domain
+ * @irq: IRQ number for mfx
+ * @irq_lock: IRQ bus lock
+ * @irqen: cache of IRQ_SRC_EN register for bus_lock
+ * @oldirqen: cache of IRQ_SRC_EN register for bus_lock
+ */
+struct mfx_priv {
+   struct device *dev;
+   struct regmap *regmap;
+   struct mfx mfx;
+   struct irq_domain *irq_domain;
+   int irq;
+   struct mutex irq_lock; /* IRQ bus lock */
+   u8 irqen;
+   u8 oldirqen;
+};
+
+#define to_mfx_priv(_mfx) container_of(_mfx, struct mfx_priv, mfx)
+
+/* MFX boot time is around 10ms, so after reset, we have to wait this delay */
+#define MFX_BOOT_TIME 10
+
+static u8 mfx_blocks_to_mask(u32 blocks)
+{
+   u8 mask = 0;
+
+   if (blocks & MFX_BLOCK_GPIO)
+   mask |= MFX_REG_SYS_CTRL_GPIO_EN;
+   else
+   mask &= ~MFX_REG_SYS_CTRL_GPIO_EN;
+
+   if (blocks & MFX_BLOCK_TS)
+   mask |= MFX_REG_SYS_CTRL_TS_EN;
+   else
+   mask &= ~MFX_REG_SYS_CTRL_TS_EN;
+
+   if (blocks & MFX_BLOCK_IDD)
+   mask |= MFX_REG_SYS_CTRL_IDD_EN;
+   else
+   mask &= ~MFX_REG_SYS_CTRL_IDD_EN;
+
+   if (blocks & MFX_BLOCK_ALTGPIO)
+   mask |= MFX_REG_SYS_CTRL_ALTGPIO_EN;
+   else
+   mask &= ~MFX_REG_SYS_CTRL_ALTGPIO_EN;
+
+   return mask;
+}
+
+static int mfx_reset(struct mfx *mfx)
+{
+   struct mfx_priv *mfx_priv = to_mfx_priv(mfx);
+   int ret;
+
+   ret = regmap_update_bits(mfx_priv->regmap, MFX_REG_SYS_CTRL,
+MFX_REG_SYS_CTRL_SWRST,
+MFX_REG_SYS_CTRL_SWRST);
+
+

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

Re: [PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

[PATCH 2/6] mfd: Add ST Multi-Function eXpander core driver

7 matches

Site Navigation

Mail list logo

Footer information