Re: [PATCH 3/3] pinctrl: at91-pio4: fix "Prefer 'unsigned int' to bare use of 'unsigned'"
On Mon, Jan 25, 2021 at 12:19:14PM +0200, Claudiu Beznea wrote:
> Fix "Prefer 'unsigned int' to bare use of 'unsigned'" checkpatch.pl
> warning.
>
> Signed-off-by: Claudiu Beznea
Acked-by: Ludovic Desroches
Thanks
> ---
> drivers/pinctrl/pinctrl-at91-pio4.c | 110
> +++-
> 1 file changed, 57 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c
> b/drivers/pinctrl/pinctrl-at91-pio4.c
> index c59ab0bfb945..0206cbfcbebb 100644
> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
> @@ -80,8 +80,8 @@
> * @sr: slew rate support
> */
> struct atmel_pioctrl_data {
> - unsigned nbanks;
> - unsigned last_bank_count;
> + unsigned int nbanks;
> + unsigned int last_bank_count;
> unsigned int sr;
> };
>
> @@ -91,11 +91,11 @@ struct atmel_group {
> };
>
> struct atmel_pin {
> - unsigned pin_id;
> - unsigned mux;
> - unsigned ioset;
> - unsigned bank;
> - unsigned line;
> + unsigned int pin_id;
> + unsigned int mux;
> + unsigned int ioset;
> + unsigned int bank;
> + unsigned int line;
> const char *device;
> };
>
> @@ -125,16 +125,16 @@ struct atmel_pin {
> struct atmel_pioctrl {
> void __iomem*reg_base;
> struct clk *clk;
> - unsignednbanks;
> + unsigned intnbanks;
> struct pinctrl_dev *pinctrl_dev;
> struct atmel_group *groups;
> const char * const *group_names;
> struct atmel_pin**pins;
> - unsignednpins;
> + unsigned intnpins;
> struct gpio_chip*gpio_chip;
> struct irq_domain *irq_domain;
> int *irqs;
> - unsigned*pm_wakeup_sources;
> + unsigned int*pm_wakeup_sources;
> struct {
> u32 imr;
> u32 odsr;
> @@ -177,11 +177,11 @@ static void atmel_gpio_irq_ack(struct irq_data *d)
>*/
> }
>
> -static int atmel_gpio_irq_set_type(struct irq_data *d, unsigned type)
> +static int atmel_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> {
> struct atmel_pioctrl *atmel_pioctrl = irq_data_get_irq_chip_data(d);
> struct atmel_pin *pin = atmel_pioctrl->pins[d->hwirq];
> - unsigned reg;
> + unsigned int reg;
>
> atmel_gpio_write(atmel_pioctrl, pin->bank, ATMEL_PIO_MSKR,
>BIT(pin->line));
> @@ -268,7 +268,7 @@ static struct irq_chip atmel_gpio_irq_chip = {
> .irq_set_wake = atmel_gpio_irq_set_wake,
> };
>
> -static int atmel_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +static int atmel_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
> {
> struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_data(chip);
>
> @@ -316,11 +316,12 @@ static void atmel_gpio_irq_handler(struct irq_desc
> *desc)
> chained_irq_exit(chip, desc);
> }
>
> -static int atmel_gpio_direction_input(struct gpio_chip *chip, unsigned
> offset)
> +static int atmel_gpio_direction_input(struct gpio_chip *chip,
> + unsigned int offset)
> {
> struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_data(chip);
> struct atmel_pin *pin = atmel_pioctrl->pins[offset];
> - unsigned reg;
> + unsigned int reg;
>
> atmel_gpio_write(atmel_pioctrl, pin->bank, ATMEL_PIO_MSKR,
>BIT(pin->line));
> @@ -331,11 +332,11 @@ static int atmel_gpio_direction_input(struct gpio_chip
> *chip, unsigned offset)
> return 0;
> }
>
> -static int atmel_gpio_get(struct gpio_chip *chip, unsigned offset)
> +static int atmel_gpio_get(struct gpio_chip *chip, unsigned int offset)
> {
> struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_data(chip);
> struct atmel_pin *pin = atmel_pioctrl->pins[offset];
> - unsigned reg;
> + unsigned int reg;
>
> reg = atmel_gpio_read(atmel_pioctrl, pin->bank, ATMEL_PIO_PDSR);
>
> @@ -369,12 +370,13 @@ static int atmel_gpio_get_multiple(struct gpio_chip
> *chip, unsigned long *mask,
> return 0;
> }
>
> -static int atmel_gpio_direction_output(struct gpio_chip *chip, unsigned
> offset,
> +static int atmel_gpio_direction_output(struct gpio_chip *chip,
> +unsigned int offset,
> int value)
> {
> struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_da
Re: [PATCH 2/3] pinctrl: at91-pio4: add support for slew-rate
On Mon, Jan 25, 2021 at 12:19:13PM +0200, Claudiu Beznea wrote:
> SAMA7G5 supports slew rate configuration. Adapt the driver for this.
> For switching frequencies lower than 50MHz the slew rate needs to
> be enabled. Since most of the pins on SAMA7G5 fall into this category
> enabled the slew rate by default.
>
> Signed-off-by: Claudiu Beznea
> ---
> drivers/pinctrl/pinctrl-at91-pio4.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c
> b/drivers/pinctrl/pinctrl-at91-pio4.c
> index d267367d94b9..c59ab0bfb945 100644
> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
> @@ -36,6 +36,7 @@
> #define ATMEL_PIO_DIR_MASK BIT(8)
> #define ATMEL_PIO_PUEN_MASK BIT(9)
> #define ATMEL_PIO_PDEN_MASK BIT(10)
> +#define ATMEL_PIO_SR_MASK BIT(11)
> #define ATMEL_PIO_IFEN_MASK BIT(12)
> #define ATMEL_PIO_IFSCEN_MASK BIT(13)
> #define ATMEL_PIO_OPD_MASK BIT(14)
> @@ -76,10 +77,12 @@
> * @nbanks: number of PIO banks
> * @last_bank_count: number of lines in the last bank (can be less than
> * the rest of the banks).
> + * @sr: slew rate support
> */
> struct atmel_pioctrl_data {
> unsigned nbanks;
> unsigned last_bank_count;
> + unsigned int sr;
Hi Claudiu,
Nitpicking: I tend to prefer a verbose name as slew_rate or even
slew_rate_support.
Otherwise,
Acked-by: Ludovic Desroches
Regards
Ludovic
> };
>
> struct atmel_group {
> @@ -117,6 +120,7 @@ struct atmel_pin {
> * @pm_suspend_backup: backup/restore register values on suspend/resume
> * @dev: device entry for the Atmel PIO controller.
> * @node: node of the Atmel PIO controller.
> + * @sr: slew rate support
> */
> struct atmel_pioctrl {
> void __iomem*reg_base;
> @@ -138,6 +142,7 @@ struct atmel_pioctrl {
> } *pm_suspend_backup;
> struct device *dev;
> struct device_node *node;
> + unsigned intsr;
> };
>
> static const char * const atmel_functions[] = {
> @@ -760,6 +765,13 @@ static int atmel_conf_pin_config_group_get(struct
> pinctrl_dev *pctldev,
> return -EINVAL;
> arg = 1;
> break;
> + case PIN_CONFIG_SLEW_RATE:
> + if (!atmel_pioctrl->sr)
> + return -EOPNOTSUPP;
> + if (!(res & ATMEL_PIO_SR_MASK))
> + return -EINVAL;
> + arg = 1;
> + break;
> case ATMEL_PIN_CONFIG_DRIVE_STRENGTH:
> if (!(res & ATMEL_PIO_DRVSTR_MASK))
> return -EINVAL;
> @@ -793,6 +805,10 @@ static int atmel_conf_pin_config_group_set(struct
> pinctrl_dev *pctldev,
> dev_dbg(pctldev->dev, "%s: pin=%u, config=0x%lx\n",
> __func__, pin_id, configs[i]);
>
> + /* Keep slew rate enabled by default. */
> + if (atmel_pioctrl->sr)
> + conf |= ATMEL_PIO_SR_MASK;
> +
> switch (param) {
> case PIN_CONFIG_BIAS_DISABLE:
> conf &= (~ATMEL_PIO_PUEN_MASK);
> @@ -850,6 +866,13 @@ static int atmel_conf_pin_config_group_set(struct
> pinctrl_dev *pctldev,
> ATMEL_PIO_SODR);
> }
> break;
> + case PIN_CONFIG_SLEW_RATE:
> + if (!atmel_pioctrl->sr)
> + break;
> + /* And remove it if explicitly requested. */
> + if (arg == 0)
> + conf &= ~ATMEL_PIO_SR_MASK;
> + break;
> case ATMEL_PIN_CONFIG_DRIVE_STRENGTH:
> switch (arg) {
> case ATMEL_PIO_DRVSTR_LO:
> @@ -901,6 +924,8 @@ static void atmel_conf_pin_config_dbg_show(struct
> pinctrl_dev *pctldev,
> seq_printf(s, "%s ", "open-drain");
> if (conf & ATMEL_PIO_SCHMITT_MASK)
> seq_printf(s, "%s ", "schmitt");
> + if (atmel_pioctrl->sr && (conf & ATMEL_PIO_SR_MASK))
> + seq_printf(s, "%s ", "slew-rate");
> if (conf & ATMEL_PIO_DRVSTR_MASK) {
> switch ((conf & ATMEL_PIO_DRVSTR_MASK) >>
> ATMEL_PIO_DRVSTR_OFFSET) {
> case ATMEL_PIO_DRVSTR_ME:
> @
Re: [PATCH 1/3] dt-bindings: pinctrl: at91-pio4: add slew-rate
On Mon, Jan 25, 2021 at 12:19:12PM +0200, Claudiu Beznea wrote: > Document slew-rate DT binding for SAMA7G5. > > Signed-off-by: Claudiu Beznea Acked-by: Ludovic Desroches Thanks > --- > .../devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt | 8 > +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt > b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt > index 265015bc0603..e2b861ce16d8 100644 > --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt > +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt > @@ -35,9 +35,11 @@ ioset settings. Use the macros from > boot/dts/-pinfunc.h file to get the > right representation of the pin. > > Optional properties: > -- GENERIC_PINCONFIG: generic pinconfig options to use, bias-disable, > -bias-pull-down, bias-pull-up, drive-open-drain, input-schmitt-enable, > -input-debounce, output-low, output-high. > +- GENERIC_PINCONFIG: generic pinconfig options to use: > + - bias-disable, bias-pull-down, bias-pull-up, drive-open-drain, > + input-schmitt-enable, input-debounce, output-low, output-high. > + - for microchip,sama7g5-pinctrl only: > + - slew-rate: 0 - disabled, 1 - enabled (default) > - atmel,drive-strength: 0 or 1 for low drive, 2 for medium drive and 3 for > high drive. The default value is low drive. > > -- > 2.7.4 >
Re: [PATCH] mmc: Assign boolean values to a bool variable
On Wed, Jan 20, 2021 at 03:39:37PM +0800, Jiapeng Zhong wrote:
> Fix the following coccicheck warnings:
>
> ./drivers/mmc/host/atmel-mci.c:2436:2-34: WARNING: Assignment
> of 0/1 to bool variable.
>
> ./drivers/mmc/host/atmel-mci.c:2425:2-20: WARNING: Assignment
> of 0/1 to bool variable.
>
> Reported-by: Abaci Robot
> Signed-off-by: Jiapeng Zhong
Acked-by: Ludovic Desroches
Thanks
> ---
> drivers/mmc/host/atmel-mci.c | 46
> ++--
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index 444bd3a..6324120 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -2401,45 +2401,45 @@ static void atmci_get_cap(struct atmel_mci *host)
> dev_info(>pdev->dev,
> "version: 0x%x\n", version);
>
> - host->caps.has_dma_conf_reg = 0;
> - host->caps.has_pdc = 1;
> - host->caps.has_cfg_reg = 0;
> - host->caps.has_cstor_reg = 0;
> - host->caps.has_highspeed = 0;
> - host->caps.has_rwproof = 0;
> - host->caps.has_odd_clk_div = 0;
> - host->caps.has_bad_data_ordering = 1;
> - host->caps.need_reset_after_xfer = 1;
> - host->caps.need_blksz_mul_4 = 1;
> - host->caps.need_notbusy_for_read_ops = 0;
> + host->caps.has_dma_conf_reg = false;
> + host->caps.has_pdc = true;
> + host->caps.has_cfg_reg = false;
> + host->caps.has_cstor_reg = false;
> + host->caps.has_highspeed = false;
> + host->caps.has_rwproof = false;
> + host->caps.has_odd_clk_div = false;
> + host->caps.has_bad_data_ordering = true;
> + host->caps.need_reset_after_xfer = true;
> + host->caps.need_blksz_mul_4 = true;
> + host->caps.need_notbusy_for_read_ops = false;
>
> /* keep only major version number */
> switch (version & 0xf00) {
> case 0x600:
> case 0x500:
> - host->caps.has_odd_clk_div = 1;
> + host->caps.has_odd_clk_div = true;
> fallthrough;
> case 0x400:
> case 0x300:
> - host->caps.has_dma_conf_reg = 1;
> - host->caps.has_pdc = 0;
> - host->caps.has_cfg_reg = 1;
> - host->caps.has_cstor_reg = 1;
> - host->caps.has_highspeed = 1;
> + host->caps.has_dma_conf_reg = true;
> + host->caps.has_pdc = false;
> + host->caps.has_cfg_reg = true;
> + host->caps.has_cstor_reg = true;
> + host->caps.has_highspeed = true;
> fallthrough;
> case 0x200:
> - host->caps.has_rwproof = 1;
> - host->caps.need_blksz_mul_4 = 0;
> - host->caps.need_notbusy_for_read_ops = 1;
> + host->caps.has_rwproof = true;
> + host->caps.need_blksz_mul_4 = false;
> + host->caps.need_notbusy_for_read_ops = true;
> fallthrough;
> case 0x100:
> - host->caps.has_bad_data_ordering = 0;
> - host->caps.need_reset_after_xfer = 0;
> + host->caps.has_bad_data_ordering = false;
> + host->caps.need_reset_after_xfer = false;
> fallthrough;
> case 0x0:
> break;
> default:
> - host->caps.has_pdc = 0;
> + host->caps.has_pdc = false;
> dev_warn(>pdev->dev,
> "Unmanaged mci version, set minimum
> capabilities\n");
> break;
> --
> 1.8.3.1
>
Re: [PATCH] pinctrl: at91-pio4: add support for fewer lines on last PIO bank
On Tue, Nov 24, 2020 at 09:31:36AM +0100, Linus Walleij wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On Fri, Nov 13, 2020 at 2:25 PM Eugen Hristev > wrote: > > > Some products, like sama7g5, do not have a full last bank of PIO lines. > > In this case for example, sama7g5 only has 8 lines for the PE bank. > > PA0-31, PB0-31, PC0-31, PD0-31, PE0-7, in total 136 lines. > > To cope with this situation, added a data attribute that is product > > dependent, > > to specify the number of lines of the last bank. > > In case this number is different from the macro ATMEL_PIO_NPINS_PER_BANK, > > adjust the total number of lines accordingly. > > This will avoid advertising 160 lines instead of the actual 136, as this > > product supports, and to avoid reading/writing to invalid register > > addresses. > > > > Signed-off-by: Eugen Hristev > > Nico/Ludovic: can you please look at this patch? I acked it one week ago but I get some nasty behaviors with my emails. Maybe you didn't receive the answer. https://lore.kernel.org/linux-gpio/20201116061549.ks6hfonyplwhknmq@sekiro/ Regards Ludovic > > Yours, > Linus Walleij
Re: [PATCH 0/3] ARM: dts: at91: add pincontrol node for USB Host
On Wed, Nov 18, 2020 at 04:26:52PM +0100, Alexandre Belloni wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Hello, > > On 18/11/2020 16:03:36+0100, Ludovic Desroches wrote: > > At first glance, there is no trivial way to register the pin range in the > > pinctrl-at91 driver. There is one driver for the pinctrl and one for the > > gpio. > > I am open to suggestions to fix it in the pinctrl-at91 driver as well if > > there > > is an elegant way (I have some in mind, but there are not) without having to > > refactor the driver. > > > > But shouldn't that driver be refactored at some point anyway? I know you > are moving away with new SoCs but it causes real issues. For example, > gpio hogs are not working, this is impacting some of your customers. > I agree, maintainance of this driver is difficult because of its design. Unfortunately, I doubt being able to hadnle a refactoring of this driver in a near future. > The other thing is the weird probe order preventing a nice cleanup of > the platform code. True. IMO, having gpio controlers probed before pinctrl is one of the reason which prevents a trivial fix. Regards Ludovic > > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Re: [PATCH 0/3] ARM: dts: at91: add pincontrol node for USB Host
Hi Cristi, Adding the gpio list. On Wed, Nov 18, 2020 at 02:00:16PM +0200, cristian.bir...@microchip.com wrote: > From: Cristian Birsan > > The pincontrol node is needed for USB Host since Linux v5.7-rc1. Without > it the driver probes but VBus is not powered because of wrong pincontrol > configuration. The patch was tested on SAM9x60EK, SAMA5D4-EK and SAMA5D3-EK. > No problem on my side with this set of patches, it's consistent with what we have. Acked-by: Ludovic Desroches I just want to share the full picture leading to this situation. You told me the breakage appears after this commit: commit 2ab73c6d8323fa1eb02c16c07c40ba2ed17da729 Author: Thierry Reding Date: Thu Mar 19 13:27:29 2020 +0100 gpio: Support GPIO controllers without pin-ranges Wake gpiochip_generic_request() call into the pinctrl helpers only if a GPIO controller had any pin-ranges assigned to it. This allows a driver to unconditionally use this helper if it supports multiple devices of which only a subset have pin-ranges assigned to them. Signed-off-by: Thierry Reding Link: https://lore.kernel.org/r/20200319122737.3063291-2-thierry.red...@gmail.com Tested-by: Vidya Sagar Signed-off-by: Linus Walleij We were used to defining pinctrl for all our pins. That is somewhat redundant when the pin is requested through the gpiolib. The pinctrl-at91 driver doesn't register any pin range. After this commit, the gpio_generic_request() fails. The consequence is if we forgot to define the pinctrl, the pin won't be muxed as a gpio. At first glance, there is no trivial way to register the pin range in the pinctrl-at91 driver. There is one driver for the pinctrl and one for the gpio. I am open to suggestions to fix it in the pinctrl-at91 driver as well if there is an elegant way (I have some in mind, but there are not) without having to refactor the driver. Regards Ludovic > Cristian Birsan (3): > ARM: dts: sam9x60: add pincontrol for USB Host > ARM: dts: at91: sama5d4_xplained: add pincontrol for USB Host > ARM: dts: at91: sama5d3_xplained: add pincontrol for USB Host > > arch/arm/boot/dts/at91-sam9x60ek.dts| 9 + > arch/arm/boot/dts/at91-sama5d3_xplained.dts | 7 +++ > arch/arm/boot/dts/at91-sama5d4_xplained.dts | 7 +++ > 3 files changed, 23 insertions(+) > > -- > 2.25.1 >
Re: [PATCH 0/9] iio: adc: at91_adc: cleanup DT bindings
On Fri, Nov 13, 2020 at 10:26:41PM +0100, Alexandre Belloni wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Hello, > > This series cleans up the at91_adc devicetree bindings. This mainly > moves back the resolution options and names and the triggers description > back in the driver. > > There are also other cleanups, like removing platform data support, this > was pending for a while. > Nice cleanup of this old and obscur binding. Reviewed-by: Ludovic Desroches Thanks. > Alexandre Belloni (8): > iio: adc: at91_adc: remove platform data > iio: adc: at91_adc: rework resolution selection > iio: adc: at91_adc: rework trigger definition > iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe > iio: adc: at91_adc: remove forward declaration > iio: adc: at91_adc: use devm_input_allocate_device > ARM: dts: at91: sama5d3: use proper ADC compatible > ARM: dts: at91: remove deprecated ADC properties > > Jonathan Cameron (1): > dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from > at91_adc.txt > > .../devicetree/bindings/iio/adc/at91_adc.txt | 83 > .../bindings/iio/adc/atmel,sama9260-adc.yaml | 121 ++ > arch/arm/boot/dts/at91sam9260.dtsi| 25 -- > arch/arm/boot/dts/at91sam9g45.dtsi| 27 -- > arch/arm/boot/dts/at91sam9rl.dtsi | 25 -- > arch/arm/boot/dts/at91sam9x5.dtsi | 28 -- > arch/arm/boot/dts/sama5d3.dtsi| 26 +- > arch/arm/boot/dts/sama5d4.dtsi| 22 - > drivers/iio/adc/at91_adc.c| 377 +++--- > include/linux/platform_data/at91_adc.h| 49 --- > 10 files changed, 259 insertions(+), 524 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt > create mode 100644 > Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml > delete mode 100644 include/linux/platform_data/at91_adc.h > > -- > 2.28.0
Re: [PATCH] pinctrl: at91-pio4: add support for fewer lines on last PIO bank
On Fri, Nov 13, 2020 at 03:24:29PM +0200, Eugen Hristev wrote:
> Some products, like sama7g5, do not have a full last bank of PIO lines.
> In this case for example, sama7g5 only has 8 lines for the PE bank.
> PA0-31, PB0-31, PC0-31, PD0-31, PE0-7, in total 136 lines.
> To cope with this situation, added a data attribute that is product dependent,
> to specify the number of lines of the last bank.
> In case this number is different from the macro ATMEL_PIO_NPINS_PER_BANK,
> adjust the total number of lines accordingly.
> This will avoid advertising 160 lines instead of the actual 136, as this
> product supports, and to avoid reading/writing to invalid register addresses.
>
> Signed-off-by: Eugen Hristev
Acked-by: Ludovic Desroches
Thanks.
> ---
> drivers/pinctrl/pinctrl-at91-pio4.c | 18 --
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c
> b/drivers/pinctrl/pinctrl-at91-pio4.c
> index 578b387100d9..d267367d94b9 100644
> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
> @@ -71,8 +71,15 @@
> /* Custom pinconf parameters */
> #define ATMEL_PIN_CONFIG_DRIVE_STRENGTH (PIN_CONFIG_END + 1)
>
> +/**
> + * struct atmel_pioctrl_data - Atmel PIO controller (pinmux + gpio) data
> struct
> + * @nbanks: number of PIO banks
> + * @last_bank_count: number of lines in the last bank (can be less than
> + * the rest of the banks).
> + */
> struct atmel_pioctrl_data {
> unsigned nbanks;
> + unsigned last_bank_count;
> };
>
> struct atmel_group {
> @@ -980,11 +987,13 @@ static const struct dev_pm_ops atmel_pctrl_pm_ops = {
> * We can have up to 16 banks.
> */
> static const struct atmel_pioctrl_data atmel_sama5d2_pioctrl_data = {
> - .nbanks = 4,
> + .nbanks = 4,
> + .last_bank_count= ATMEL_PIO_NPINS_PER_BANK,
> };
>
> static const struct atmel_pioctrl_data microchip_sama7g5_pioctrl_data = {
> - .nbanks = 5,
> + .nbanks = 5,
> + .last_bank_count= 8, /* sama7g5 has only PE0 to PE7 */
> };
>
> static const struct of_device_id atmel_pctrl_of_match[] = {
> @@ -1025,6 +1034,11 @@ static int atmel_pinctrl_probe(struct platform_device
> *pdev)
> atmel_pioctrl_data = match->data;
> atmel_pioctrl->nbanks = atmel_pioctrl_data->nbanks;
> atmel_pioctrl->npins = atmel_pioctrl->nbanks * ATMEL_PIO_NPINS_PER_BANK;
> + /* if last bank has limited number of pins, adjust accordingly */
> + if (atmel_pioctrl_data->last_bank_count != ATMEL_PIO_NPINS_PER_BANK) {
> + atmel_pioctrl->npins -= ATMEL_PIO_NPINS_PER_BANK;
> + atmel_pioctrl->npins += atmel_pioctrl_data->last_bank_count;
> + }
>
> atmel_pioctrl->reg_base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(atmel_pioctrl->reg_base))
> --
> 2.25.1
>
Re: [PATCH 4.19] dmaengine: at_hdmac: Fix memory leak
On Sun, Sep 20, 2020 at 10:28:38AM +0200, Pavel Machek wrote:
> Date: Sun, 20 Sep 2020 10:28:38 +0200
> From: Pavel Machek
> To: linux-kernel@vger.kernel.org, dmaeng...@vger.kernel.org,
> linux-arm-ker...@lists.infradead.org, dan.j.willi...@intel.com,
> vk...@kernel.org, ludovic.desroc...@microchip.com, sta...@vger.kernel.org,
> Greg KH
> Subject: [PATCH 4.19] dmaengine: at_hdmac: Fix memory leak
>
>
> This fixes memory leak in at_hdmac. Mainline does not have the same
> problem.
>
> Signed-off-by: Pavel Machek (CIP)
Acked-by: Ludovic Desroches
Thanks.
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 86427f6ba78c..0847b2055857 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -1714,8 +1714,10 @@ static struct dma_chan *at_dma_xlate(struct
> of_phandle_args *dma_spec,
> atslave->dma_dev = _pdev->dev;
>
> chan = dma_request_channel(mask, at_dma_filter, atslave);
> - if (!chan)
> + if (!chan) {
> + kfree(atslave);
> return NULL;
> + }
>
> atchan = to_at_dma_chan(chan);
> atchan->per_if = dma_spec->args[0] & 0xff;
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH 1/2] dt-bindings: pinctrl: at91-pio4: add microchip,sama7g5
On Thu, Sep 17, 2020 at 04:12:56PM +0300, Eugen Hristev wrote: > Add compatible string for microchip sama7g5 SoC. > > Signed-off-by: Eugen Hristev Acked-by: Ludovic Desroches Thanks Ludovic > --- > .../devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt > b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt > index 04d16fb69eb7..265015bc0603 100644 > --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt > +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt > @@ -4,7 +4,9 @@ The Atmel PIO4 controller is used to select the function of a > pin and to > configure it. > > Required properties: > -- compatible: "atmel,sama5d2-pinctrl". > +- compatible: > + "atmel,sama5d2-pinctrl" > + "microchip,sama7g5-pinctrl" > - reg: base address and length of the PIO controller. > - interrupts: interrupt outputs from the controller, one for each bank. > - interrupt-controller: mark the device node as an interrupt controller. > -- > 2.25.1 > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH 2/2] pinctrl: at91-pio4: add support for sama7g5 SoC
On Thu, Sep 17, 2020 at 04:12:57PM +0300, Eugen Hristev wrote:
> Add support for sama7g5 pinctrl block, which has 5 PIO banks.
>
> Signed-off-by: Eugen Hristev
Acked-by: Ludovic Desroches
Thanks.
Ludovic
> ---
> drivers/pinctrl/pinctrl-at91-pio4.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c
> b/drivers/pinctrl/pinctrl-at91-pio4.c
> index 1c852293cb96..9f62152d0a62 100644
> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
> @@ -999,10 +999,17 @@ static const struct atmel_pioctrl_data
> atmel_sama5d2_pioctrl_data = {
> .nbanks = 4,
> };
>
> +static const struct atmel_pioctrl_data microchip_sama7g5_pioctrl_data = {
> + .nbanks = 5,
> +};
> +
> static const struct of_device_id atmel_pctrl_of_match[] = {
> {
> .compatible = "atmel,sama5d2-pinctrl",
> .data = _sama5d2_pioctrl_data,
> + }, {
> + .compatible = "microchip,sama7g5-pinctrl",
> + .data = _sama7g5_pioctrl_data,
> }, {
> /* sentinel */
> }
> --
> 2.25.1
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH 05/10] gpio: gpio-sama5d2-piobu: Demote all kerneldoc headers to basic comment blocks
On Tue, Jun 30, 2020 at 02:33:40PM +0100, Lee Jones wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> No attempt has been made to provide proper descriptions for each of
> the function arguments throughout the file. Simply demote all
> kerneldoc headers to basic function headers.
>
> Fixes the following W=1 kernel build warnings:
>
> drivers/gpio/gpio-sama5d2-piobu.c:59: warning: Function parameter or member
> 'chip' not described in 'sama5d2_piobu_setup_pin'
> drivers/gpio/gpio-sama5d2-piobu.c:59: warning: Function parameter or member
> 'pin' not described in 'sama5d2_piobu_setup_pin'
> drivers/gpio/gpio-sama5d2-piobu.c:81: warning: Function parameter or member
> 'chip' not described in 'sama5d2_piobu_write_value'
> drivers/gpio/gpio-sama5d2-piobu.c:81: warning: Function parameter or member
> 'pin' not described in 'sama5d2_piobu_write_value'
> drivers/gpio/gpio-sama5d2-piobu.c:81: warning: Function parameter or member
> 'mask' not described in 'sama5d2_piobu_write_value'
> drivers/gpio/gpio-sama5d2-piobu.c:81: warning: Function parameter or member
> 'value' not described in 'sama5d2_piobu_write_value'
> drivers/gpio/gpio-sama5d2-piobu.c:97: warning: Function parameter or member
> 'chip' not described in 'sama5d2_piobu_read_value'
> drivers/gpio/gpio-sama5d2-piobu.c:97: warning: Function parameter or member
> 'pin' not described in 'sama5d2_piobu_read_value'
> drivers/gpio/gpio-sama5d2-piobu.c:97: warning: Function parameter or member
> 'mask' not described in 'sama5d2_piobu_read_value'
> drivers/gpio/gpio-sama5d2-piobu.c:116: warning: Function parameter or member
> 'chip' not described in 'sama5d2_piobu_get_direction'
> drivers/gpio/gpio-sama5d2-piobu.c:116: warning: Function parameter or member
> 'pin' not described in 'sama5d2_piobu_get_direction'
> drivers/gpio/gpio-sama5d2-piobu.c:131: warning: Function parameter or member
> 'chip' not described in 'sama5d2_piobu_direction_input'
> drivers/gpio/gpio-sama5d2-piobu.c:131: warning: Function parameter or member
> 'pin' not described in 'sama5d2_piobu_direction_input'
> drivers/gpio/gpio-sama5d2-piobu.c:140: warning: Function parameter or member
> 'chip' not described in 'sama5d2_piobu_direction_output'
> drivers/gpio/gpio-sama5d2-piobu.c:140: warning: Function parameter or member
> 'pin' not described in 'sama5d2_piobu_direction_output'
> drivers/gpio/gpio-sama5d2-piobu.c:140: warning: Function parameter or member
> 'value' not described in 'sama5d2_piobu_direction_output'
> drivers/gpio/gpio-sama5d2-piobu.c:154: warning: Function parameter or member
> 'chip' not described in 'sama5d2_piobu_get'
> drivers/gpio/gpio-sama5d2-piobu.c:154: warning: Function parameter or member
> 'pin' not described in 'sama5d2_piobu_get'
> drivers/gpio/gpio-sama5d2-piobu.c:174: warning: Function parameter or member
> 'chip' not described in 'sama5d2_piobu_set'
> drivers/gpio/gpio-sama5d2-piobu.c:174: warning: Function parameter or member
> 'pin' not described in 'sama5d2_piobu_set'
> drivers/gpio/gpio-sama5d2-piobu.c:174: warning: Function parameter or member
> 'value' not described in 'sama5d2_piobu_set'
>
> Cc: Ludovic Desroches
> Cc: Andrei Stefanescu
> Signed-off-by: Lee Jones
Acked-by: Ludovic Desroches
Thanks.
> ---
> drivers/gpio/gpio-sama5d2-piobu.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpio/gpio-sama5d2-piobu.c
> b/drivers/gpio/gpio-sama5d2-piobu.c
> index 4d47b2c411868..b7c9506581701 100644
> --- a/drivers/gpio/gpio-sama5d2-piobu.c
> +++ b/drivers/gpio/gpio-sama5d2-piobu.c
> @@ -49,7 +49,7 @@ struct sama5d2_piobu {
> struct regmap *regmap;
> };
>
> -/**
> +/*
> * sama5d2_piobu_setup_pin() - prepares a pin for set_direction call
> *
> * Do not consider pin for tamper detection (normal and backup modes)
> @@ -73,7 +73,7 @@ static int sama5d2_piobu_setup_pin(struct gpio_chip *chip,
> unsigned int pin)
> return regmap_update_bits(piobu->regmap, PIOBU_WKPR, mask, 0);
> }
>
> -/**
> +/*
> * sama5d2_piobu_write_value() - writes value & mask at the pin's PIOBU
> register
> */
> static int sama5d2_piobu_write_value(struct gpio_chip *chip, unsigned int
> pin,
> @@ -88,7 +88,7 @@ static int sama5d2_piobu_write_value(struct gpio_chip
> *chip, unsigned int pin,
> return regmap_update_bits(piobu->regmap, reg, mask, value);
> }
>
> -/**
> +/*
> * sama5d2_piobu_read_value() - read the value with masking from the pin's
> PIOBU
> * register
> */
> @@ -108,7 +108,7 @@ static int sama5d2_piobu_read_value(struct gpio_ch
Re: [PATCH] iio: Kconfig: at91_adc: add COMPILE_TEST dependency to driver
On Sun, May 31, 2020 at 03:40:17PM +0100, Jonathan Cameron wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On Mon, 25 May 2020 13:27:44 +0300 > Alexandru Ardelean wrote: > > > Since changes can come from all sort of places, it may make sense to have > > this symbol as a dependency to make sure that the 'make allmodconfig' && > > 'make allyesconfig' build rules cover this driver as well for a > > compile-build/test. > > > > It seemed useful [recently] when trying to apply a change for this. > > > > Signed-off-by: Alexandru Ardelean > Makes sense. Given this sort of change can expose weird an wonderful > dependencies that were previously hidden, I'll be wanting an > ack from at91 people. Agree. Acked-by: Ludovic Desroches Regards Ludovic > > Jonathan > > > --- > > drivers/iio/adc/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index c48c0005..c1f4c0aec265 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -294,7 +294,7 @@ config ASPEED_ADC > > > > config AT91_ADC > > tristate "Atmel AT91 ADC" > > - depends on ARCH_AT91 > > + depends on ARCH_AT91 || COMPILE_TEST > > depends on INPUT && SYSFS > > select IIO_BUFFER > > select IIO_TRIGGERED_BUFFER >
Re: [PATCH] dmaengine: at_hdmac: Replace zero-length array with flexible-array
On Thu, May 07, 2020 at 02:00:38PM -0500, Gustavo A. R. Silva wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be hiding
> some bugs. So, this work (flexible-array member conversions) will also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva
Acked-by: Ludovic Desroches
Thanks
> ---
> drivers/dma/at_hdmac_regs.h |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
> index 397692e937b3..80fc2fe8c77e 100644
> --- a/drivers/dma/at_hdmac_regs.h
> +++ b/drivers/dma/at_hdmac_regs.h
> @@ -331,7 +331,7 @@ struct at_dma {
> struct dma_pool *dma_desc_pool;
> struct dma_pool *memset_pool;
> /* AT THE END channels table */
> - struct at_dma_chan chan[0];
> + struct at_dma_chan chan[];
> };
>
> #definedma_readl(atdma, name) \
>
Re: [PATCH] dmaengine: at_xdmac: Replace zero-length array with flexible-array
On Thu, May 07, 2020 at 02:00:46PM -0500, Gustavo A. R. Silva wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be hiding
> some bugs. So, this work (flexible-array member conversions) will also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva
Acked-by: Ludovic Desroches
Ludovic Desroches
> ---
> drivers/dma/at_xdmac.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index bb0eaf38b594..fd92f048c491 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -212,7 +212,7 @@ struct at_xdmac {
> struct clk *clk;
> u32 save_gim;
> struct dma_pool *at_xdmac_desc_pool;
> - struct at_xdmac_chanchan[0];
> + struct at_xdmac_chanchan[];
> };
>
>
>
Re: [PATCH v2 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties
On Fri, Oct 18, 2019 at 12:56:14PM +0200, Ulf Hansson wrote:
> On Fri, 11 Oct 2019 at 12:33, Ludovic Desroches
> wrote:
> >
> > There is a new compatible string for the SAM9X60 sdhci device. It involves
> > an update of the properties about the clocks stuff.
> >
> > Signed-off-by: Ludovic Desroches
>
> This doesn't apply any more, can you please re-spin it?
Sure, I'll rebase these patches and send a v2
Regards
Ludovic
>
> Kind regards
> Uffe
>
>
> > ---
> >
> > Changes:
> > - v2: remove the extra example and fix node label
> >
> > This patch conflicts with Nicolas' one: "dt-bindings: sdhci-of-at91: add
> > the microchip,sdcal-inverted property". Let me know which one has to be
> > rebased or you can handle it.
> >
> > Ludovic
> >
> >
> > .../devicetree/bindings/mmc/sdhci-atmel.txt | 15 ---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> > b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> > index 1b662d7171a0..5d541ad4d4eb 100644
> > --- a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> > +++ b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> > @@ -5,17 +5,26 @@ Documentation/devicetree/bindings/mmc/mmc.txt and the
> > properties used by the
> > sdhci-of-at91 driver.
> >
> > Required properties:
> > -- compatible: Must be "atmel,sama5d2-sdhci".
> > +- compatible: Must be "atmel,sama5d2-sdhci" or
> > "microchip,sam9x60-sdhci".
> > - clocks: Phandlers to the clocks.
> > -- clock-names: Must be "hclock", "multclk", "baseclk";
> > +- clock-names: Must be "hclock", "multclk", "baseclk" for
> > + "atmel,sama5d2-sdhci".
> > + Must be "hclock", "multclk" for
> > "microchip,sam9x60-sdhci".
> > +
> > +Optional properties:
> > +- assigned-clocks: The same with "multclk".
> > +- assigned-clock-rates The rate of "multclk" in order to not rely on the
> > + gck configuration set by previous components.
> >
> >
> > Example:
> >
> > -sdmmc0: sdio-host@a000 {
> > +mmc0: sdio-host@a000 {
> > compatible = "atmel,sama5d2-sdhci";
> > reg = <0xa000 0x300>;
> > interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>;
> > clocks = <_hclk>, <_gclk>, <>;
> > clock-names = "hclock", "multclk", "baseclk";
> > + assigned-clocks = <_gclk>;
> > + assigned-clock-rates = <48000>;
> > };
> > --
> > 2.23.0
> >
Re: [PATCH 2/2] mmc: sdhci-of-at91: add DT property to enable calibration on full reset
On Tue, Oct 08, 2019 at 02:34:32PM +0200, Nicolas Ferre wrote:
> Add a property to keep the analog calibration cell powered.
> This feature is specific to the Microchip SDHCI IP and outside
> of the standard SDHCI register map.
>
> By always keeping it on, after a full reset sequence, we make sure
> that this feature is activated and not disabled.
>
> We expose a hardware property to the DT as this feature can be used
> to adapt SDHCI behavior vs. how the SDCAL SoC pin is connected
> on the board.
>
> Note that managing properly this property would reduce
> power consumption on some SAMA5D2 SiP revisions.
>
> Signed-off-by: Nicolas Ferre
Acked-by: Ludovic Desroches
Thanks
Ludovic
> ---
> drivers/mmc/host/sdhci-of-at91.c | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c
> b/drivers/mmc/host/sdhci-of-at91.c
> index e7d1920729fb..9571c4a882a9 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -27,6 +27,9 @@
> #define SDMMC_CACR 0x230
> #define SDMMC_CACR_CAPWREN BIT(0)
> #define SDMMC_CACR_KEY (0x46 << 8)
> +#define SDMMC_CALCR 0x240
> +#define SDMMC_CALCR_EN BIT(0)
> +#define SDMMC_CALCR_ALWYSON BIT(4)
>
> #define SDHCI_AT91_PRESET_COMMON_CONF0x400 /* drv type B,
> programmable clock mode */
>
> @@ -35,6 +38,7 @@ struct sdhci_at91_priv {
> struct clk *gck;
> struct clk *mainck;
> bool restore_needed;
> + bool cal_always_on;
> };
>
> static void sdhci_at91_set_force_card_detect(struct sdhci_host *host)
> @@ -116,10 +120,17 @@ static void sdhci_at91_set_uhs_signaling(struct
> sdhci_host *host,
>
> static void sdhci_at91_reset(struct sdhci_host *host, u8 mask)
> {
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> sdhci_reset(host, mask);
>
> if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
> sdhci_at91_set_force_card_detect(host);
> +
> + if (priv->cal_always_on && (mask & SDHCI_RESET_ALL))
> + sdhci_writel(host, SDMMC_CALCR_ALWYSON | SDMMC_CALCR_EN,
> + SDMMC_CALCR);
> }
>
> static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
> @@ -345,6 +356,14 @@ static int sdhci_at91_probe(struct platform_device *pdev)
>
> priv->restore_needed = false;
>
> + /*
> + * if SDCAL pin is wrongly connected, we must enable
> + * the analog calibration cell permanently.
> + */
> + priv->cal_always_on =
> + device_property_read_bool(>dev,
> + "microchip,sdcal-inverted");
> +
> ret = mmc_of_parse(host->mmc);
> if (ret)
> goto clocks_disable_unprepare;
> --
> 2.17.1
>
[PATCH v2 3/3] ARM: dts: at91: sama5d2: set the sdmmc gclk frequency
Set the frequency of the generated clock used by sdmmc devices in order to not rely on the configuration done by previous components. Signed-off-by: Ludovic Desroches --- Changes: -v2: none arch/arm/boot/dts/sama5d2.dtsi | 4 1 file changed, 4 insertions(+) diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi index 565204816e34..7665263af907 100644 --- a/arch/arm/boot/dts/sama5d2.dtsi +++ b/arch/arm/boot/dts/sama5d2.dtsi @@ -300,6 +300,8 @@ interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>; clocks = < PMC_TYPE_PERIPHERAL 31>, < PMC_TYPE_GCK 31>, < PMC_TYPE_CORE PMC_MAIN>; clock-names = "hclock", "multclk", "baseclk"; + assigned-clocks = < PMC_TYPE_GCK 31>; + assigned-clock-rates = <48000>; status = "disabled"; }; @@ -309,6 +311,8 @@ interrupts = <32 IRQ_TYPE_LEVEL_HIGH 0>; clocks = < PMC_TYPE_PERIPHERAL 32>, < PMC_TYPE_GCK 32>, < PMC_TYPE_CORE PMC_MAIN>; clock-names = "hclock", "multclk", "baseclk"; + assigned-clocks = < PMC_TYPE_GCK 32>; + assigned-clock-rates = <48000>; status = "disabled"; }; -- 2.23.0
[PATCH v2 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device
In the SAM9x60 SoC, there are only two clocks instead of three for the
SDHCI device. The base clk is no longer provided, it is generated
internally from the mult clk.
The values of the base clk and mul in the capabilities registers may not
reflect the reality as the mult clk is a programmable clock which can take
several rates. As we can't trust those values, take them from the clock
tree and update the capabilities according to.
As we can have the same pitfall, in some cases, with the SAMA5D2 Soc,
stop relying on capabilities too.
Signed-off-by: Ludovic Desroches
---
Changes:
- v2: write caps0
drivers/mmc/host/sdhci-of-at91.c | 105 +--
1 file changed, 58 insertions(+), 47 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index e7d1920729fb..9a1ae16a5417 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -30,7 +30,14 @@
#define SDHCI_AT91_PRESET_COMMON_CONF 0x400 /* drv type B, programmable clock
mode */
+struct sdhci_at91_soc_data {
+ const struct sdhci_pltfm_data *pdata;
+ bool baseclk_is_generated_internally;
+ unsigned int divider_for_baseclk;
+};
+
struct sdhci_at91_priv {
+ const struct sdhci_at91_soc_data *soc_data;
struct clk *hclock;
struct clk *gck;
struct clk *mainck;
@@ -130,12 +137,24 @@ static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
.set_power = sdhci_at91_set_power,
};
-static const struct sdhci_pltfm_data soc_data_sama5d2 = {
+static const struct sdhci_pltfm_data sdhci_sama5d2_pdata = {
.ops = _at91_sama5d2_ops,
};
+static const struct sdhci_at91_soc_data soc_data_sama5d2 = {
+ .pdata = _sama5d2_pdata,
+ .baseclk_is_generated_internally = false,
+};
+
+static const struct sdhci_at91_soc_data soc_data_sam9x60 = {
+ .pdata = _sama5d2_pdata,
+ .baseclk_is_generated_internally = true,
+ .divider_for_baseclk = 2,
+};
+
static const struct of_device_id sdhci_at91_dt_match[] = {
{ .compatible = "atmel,sama5d2-sdhci", .data = _data_sama5d2 },
+ { .compatible = "microchip,sam9x60-sdhci", .data = _data_sam9x60 },
{}
};
MODULE_DEVICE_TABLE(of, sdhci_at91_dt_match);
@@ -145,50 +164,37 @@ static int sdhci_at91_set_clks_presets(struct device *dev)
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host);
- int ret;
unsigned intcaps0, caps1;
unsigned intclk_base, clk_mul;
- unsigned intgck_rate, real_gck_rate;
+ unsigned intgck_rate, clk_base_rate;
unsigned intpreset_div;
- /*
-* The mult clock is provided by as a generated clock by the PMC
-* controller. In order to set the rate of gck, we have to get the
-* base clock rate and the clock mult from capabilities.
-*/
clk_prepare_enable(priv->hclock);
caps0 = readl(host->ioaddr + SDHCI_CAPABILITIES);
caps1 = readl(host->ioaddr + SDHCI_CAPABILITIES_1);
- clk_base = (caps0 & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
- clk_mul = (caps1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
- gck_rate = clk_base * 100 * (clk_mul + 1);
- ret = clk_set_rate(priv->gck, gck_rate);
- if (ret < 0) {
- dev_err(dev, "failed to set gck");
- clk_disable_unprepare(priv->hclock);
- return ret;
- }
- /*
-* We need to check if we have the requested rate for gck because in
-* some cases this rate could be not supported. If it happens, the rate
-* is the closest one gck can provide. We have to update the value
-* of clk mul.
-*/
- real_gck_rate = clk_get_rate(priv->gck);
- if (real_gck_rate != gck_rate) {
- clk_mul = real_gck_rate / (clk_base * 100) - 1;
- caps1 &= (~SDHCI_CLOCK_MUL_MASK);
- caps1 |= ((clk_mul << SDHCI_CLOCK_MUL_SHIFT) &
- SDHCI_CLOCK_MUL_MASK);
- /* Set capabilities in r/w mode. */
- writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN,
- host->ioaddr + SDMMC_CACR);
- writel(caps1, host->ioaddr + SDHCI_CAPABILITIES_1);
- /* Set capabilities in ro mode. */
- writel(0, host->ioaddr + SDMMC_CACR);
- dev_info(dev, "update clk mul to %u as gck rate is %u Hz\n",
-clk_mul, real_gck_rate);
- }
+
+ gck_rate = clk_get_rate(priv->gck);
+ if (priv->soc_data->baseclk_is_generated_internally)
+
[PATCH v2 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties
There is a new compatible string for the SAM9X60 sdhci device. It involves
an update of the properties about the clocks stuff.
Signed-off-by: Ludovic Desroches
---
Changes:
- v2: remove the extra example and fix node label
This patch conflicts with Nicolas' one: "dt-bindings: sdhci-of-at91: add
the microchip,sdcal-inverted property". Let me know which one has to be
rebased or you can handle it.
Ludovic
.../devicetree/bindings/mmc/sdhci-atmel.txt | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
index 1b662d7171a0..5d541ad4d4eb 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
@@ -5,17 +5,26 @@ Documentation/devicetree/bindings/mmc/mmc.txt and the
properties used by the
sdhci-of-at91 driver.
Required properties:
-- compatible: Must be "atmel,sama5d2-sdhci".
+- compatible: Must be "atmel,sama5d2-sdhci" or
"microchip,sam9x60-sdhci".
- clocks: Phandlers to the clocks.
-- clock-names: Must be "hclock", "multclk", "baseclk";
+- clock-names: Must be "hclock", "multclk", "baseclk" for
+ "atmel,sama5d2-sdhci".
+ Must be "hclock", "multclk" for
"microchip,sam9x60-sdhci".
+
+Optional properties:
+- assigned-clocks: The same with "multclk".
+- assigned-clock-rates The rate of "multclk" in order to not rely on the
+ gck configuration set by previous components.
Example:
-sdmmc0: sdio-host@a000 {
+mmc0: sdio-host@a000 {
compatible = "atmel,sama5d2-sdhci";
reg = <0xa000 0x300>;
interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>;
clocks = <_hclk>, <_gclk>, <>;
clock-names = "hclock", "multclk", "baseclk";
+ assigned-clocks = <_gclk>;
+ assigned-clock-rates = <48000>;
};
--
2.23.0
Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery
On Wed, Oct 09, 2019 at 04:01:47PM +0200, Alexandre Belloni wrote: > > On 09/10/2019 15:55:00+0200, Ludovic Desroches wrote: > > On Wed, Oct 02, 2019 at 04:46:56PM +0200, Kamel Bouhara wrote: > > > External E-Mail > > > > > > > > > Implement i2c bus recovery when slaves devices might hold SDA low. > > > In this case re-assign SCL/SDA to gpios and issue 9 dummy clock pulses > > > until the slave release SDA. > > > > > > > Hi Kamel, > > > > Thanks for adding this new feature. As I see patches only for sama5d3 and > > sama5d4, I assume it has not been tested with a sama5d2, isn't it? > > > > I there a point having it on sama5d2 as the controller already supports > this feature? > Right, I was focused on pinctrl and forget we have this feature supported by the IP. > > I doubt it works with a sama5d2 because of the pinctrl. I also wonder if it > > can > > work if we add .strict = true to pinmux_ops which is something plan for the > > future... > > > > I don't see why it wouldn't work with strict as this is switching muxing > properly instead of using the pins for two functions at the same time. > Not sure devm_gpiod_get won't fail with strict. Ludovic
Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery
On Wed, Oct 02, 2019 at 04:46:56PM +0200, Kamel Bouhara wrote:
> External E-Mail
>
>
> Implement i2c bus recovery when slaves devices might hold SDA low.
> In this case re-assign SCL/SDA to gpios and issue 9 dummy clock pulses
> until the slave release SDA.
>
Hi Kamel,
Thanks for adding this new feature. As I see patches only for sama5d3 and
sama5d4, I assume it has not been tested with a sama5d2, isn't it?
I doubt it works with a sama5d2 because of the pinctrl. I also wonder if it can
work if we add .strict = true to pinmux_ops which is something plan for the
future...
Are you able to test these points? It would be nice to be aware of
possible side effects.
Regards
Ludovic
> Signed-off-by: Kamel Bouhara
> ---
> drivers/i2c/busses/i2c-at91-master.c | 63
> drivers/i2c/busses/i2c-at91.h| 8
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-at91-master.c
> b/drivers/i2c/busses/i2c-at91-master.c
> index a3fcc35ffd3b..df5bb93f952d 100644
> --- a/drivers/i2c/busses/i2c-at91-master.c
> +++ b/drivers/i2c/busses/i2c-at91-master.c
> @@ -18,11 +18,13 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -768,6 +770,63 @@ static int at91_twi_configure_dma(struct at91_twi_dev
> *dev, u32 phy_addr)
> return ret;
> }
>
> +static void at91_prepare_twi_recovery(struct i2c_adapter *adap)
> +{
> + struct at91_twi_dev *dev = i2c_get_adapdata(adap);
> +
> + pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_gpio);
> +}
> +
> +static void at91_unprepare_twi_recovery(struct i2c_adapter *adap)
> +{
> + struct at91_twi_dev *dev = i2c_get_adapdata(adap);
> +
> + pinctrl_select_state(dev->pinctrl, dev->pinctrl_pins_default);
> +}
> +
> +static int at91_init_twi_recovery_info(struct platform_device *pdev,
> +struct at91_twi_dev *dev)
> +{
> + struct i2c_bus_recovery_info *rinfo = >rinfo;
> +
> + dev->pinctrl = devm_pinctrl_get(>dev);
> + if (!dev->pinctrl || IS_ERR(dev->pinctrl)) {
> + dev_info(dev->dev, "can't get pinctrl, bus recovery not
> supported\n");
> + return PTR_ERR(dev->pinctrl);
> + }
> +
> + dev->pinctrl_pins_default = pinctrl_lookup_state(dev->pinctrl,
> + PINCTRL_STATE_DEFAULT);
> + dev->pinctrl_pins_gpio = pinctrl_lookup_state(dev->pinctrl,
> + "gpio");
> + rinfo->sda_gpiod = devm_gpiod_get(>dev, "sda", GPIOD_IN);
> + if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + rinfo->scl_gpiod = devm_gpiod_get(>dev, "scl",
> + GPIOD_OUT_HIGH_OPEN_DRAIN);
> + if (PTR_ERR(rinfo->scl_gpiod) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + if (IS_ERR(rinfo->sda_gpiod) ||
> +IS_ERR(rinfo->scl_gpiod) ||
> +IS_ERR(dev->pinctrl_pins_default) ||
> +IS_ERR(dev->pinctrl_pins_gpio)) {
> + dev_info(>dev, "recovery information incomplete\n");
> + return -EINVAL;
> + }
> +
> + dev_info(>dev, "using scl%s for recovery\n",
> + rinfo->sda_gpiod ? ",sda" : "");
> +
> + rinfo->prepare_recovery = at91_prepare_twi_recovery;
> + rinfo->unprepare_recovery = at91_unprepare_twi_recovery;
> + rinfo->recover_bus = i2c_generic_scl_recovery;
> + dev->adapter.bus_recovery_info = rinfo;
> +
> + return 0;
> +}
> +
> int at91_twi_probe_master(struct platform_device *pdev,
> u32 phy_addr, struct at91_twi_dev *dev)
> {
> @@ -795,6 +854,10 @@ int at91_twi_probe_master(struct platform_device *pdev,
>
> at91_calc_twi_clock(dev);
>
> + rc = at91_init_twi_recovery_info(pdev, dev);
> + if (rc == -EPROBE_DEFER)
> + return rc;
> +
> dev->adapter.algo = _twi_algorithm;
> dev->adapter.quirks = _twi_quirks;
>
> diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
> index 499b506f6128..b89dab55e776 100644
> --- a/drivers/i2c/busses/i2c-at91.h
> +++ b/drivers/i2c/busses/i2c-at91.h
> @@ -141,6 +141,10 @@ struct at91_twi_dev {
> u32 fifo_size;
> struct at91_twi_dma dma;
> bool slave_detected;
> + struct i2c_bus_recovery_info rinfo;
> + struct pinctrl *pinctrl;
> + struct pinctrl_state *pinctrl_pins_default;
> + struct pinctrl_state *pinctrl_pins_gpio;
> #ifdef CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL
> unsigned smr;
> struct i2c_client *slave;
> @@ -158,6 +162,10 @@ void at91_init_twi_bus_master(struct at91_twi_dev *dev);
> int at91_twi_probe_master(struct platform_device *pdev, u32 phy_addr,
> struct at91_twi_dev *dev);
>
> +void at91_twi_prepare_recovery(struct i2c_adapter
Re: [PATCH] dmaengine: at_xdmac: Use devm_platform_ioremap_resource() in at_xdmac_probe()
On Sun, Sep 22, 2019 at 10:48:20AM +0200, Markus Elfring wrote:
>
> From: Markus Elfring
> Date: Sun, 22 Sep 2019 10:37:31 +0200
>
> Simplify this function implementation by using a known wrapper function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring
Acked-by: Ludovic Desroches
Thanks
> ---
> drivers/dma/at_xdmac.c | 7 +--
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index b58ac720d9a1..f71c9f77d405 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -1957,21 +1957,16 @@ static int atmel_xdmac_resume(struct device *dev)
>
> static int at_xdmac_probe(struct platform_device *pdev)
> {
> - struct resource *res;
> struct at_xdmac *atxdmac;
> int irq, size, nr_channels, i, ret;
> void __iomem*base;
> u32 reg;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res)
> - return -EINVAL;
> -
> irq = platform_get_irq(pdev, 0);
> if (irq < 0)
> return irq;
>
> - base = devm_ioremap_resource(>dev, res);
> + base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> --
> 2.23.0
>
Re: [PATCH 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties
On Mon, Sep 30, 2019 at 09:56:13AM -0500, Rob Herring wrote:
> On Thu, Sep 12, 2019 at 10:09:06PM +0200, Ludovic Desroches wrote:
> > There is a new compatible string for the SAM9X60 sdhci device. It involves
> > an update of the properties about the clocks stuff.
> >
> > Signed-off-by: Ludovic Desroches
> > ---
> > .../devicetree/bindings/mmc/sdhci-atmel.txt | 25 ---
> > 1 file changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> > b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> > index 1b662d7171a0..364ceea330b6 100644
> > --- a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> > +++ b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> > @@ -5,12 +5,19 @@ Documentation/devicetree/bindings/mmc/mmc.txt and the
> > properties used by the
> > sdhci-of-at91 driver.
> >
> > Required properties:
> > -- compatible: Must be "atmel,sama5d2-sdhci".
> > +- compatible: Must be "atmel,sama5d2-sdhci" or
> > "microchip,sam9x60-sdhci".
> > - clocks: Phandlers to the clocks.
> > -- clock-names: Must be "hclock", "multclk", "baseclk";
> > +- clock-names: Must be "hclock", "multclk", "baseclk" for
> > + "atmel,sama5d2-sdhci".
> > + Must be "hclock", "multclk" for
> > "microchip,sam9x60-sdhci".
> >
> > +Optional properties:
> > +- assigned-clocks: The same with "multclk".
> > +- assigned-clock-rates The rate of "multclk" in order to not rely on
> > the
> > + gck configuration set by previous components.
> >
> > -Example:
> > +
> > +Examples:
> >
> > sdmmc0: sdio-host@a000 {
> > compatible = "atmel,sama5d2-sdhci";
> > @@ -18,4 +25,16 @@ sdmmc0: sdio-host@a000 {
> > interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>;
> > clocks = <_hclk>, <_gclk>, <>;
> > clock-names = "hclock", "multclk", "baseclk";
> > + assigned-clocks = <_gclk>;
> > + assigned-clock-rates = <48000>;
> > +};
> > +
> > +sdmmc0: sdio-host@8000 {
>
> mmc@...
>
> Though I don't see much value in a second example. Examples are not a
> complete enumeration of all possible dts entries.
>
Ok, I'll skip this example.
Ludovic
> > + compatible = "microchip,sam9x60-sdhci";
> > + reg = <0x8000 0x300>;
> > + interrupts = <12 IRQ_TYPE_LEVEL_HIGH 0>;
> > + clocks = < PMC_TYPE_PERIPHERAL 12>, < PMC_TYPE_GCK 12>;
> > + clock-names = "hclock", "multclk";
> > + assigned-clocks = < PMC_TYPE_GCK 12>;
> > + assigned-clock-rates = <1>;
> > };
> > --
> > 2.23.0
> >
Re: [PATCH 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device
On Fri, Sep 20, 2019 at 11:34:50AM +0200, Eugen Hristev - M18282 wrote:
>
>
> On 12.09.2019 23:09, Ludovic Desroches wrote:
>
> >
> > In the SAM9x60 SoC, there are only two clocks instead of three for the
> > SDHCI device. The base clk is no longer provided, it is generated
> > internally from the mult clk.
> >
> > The values of the base clk and mul in the capabilities registers may not
> > reflect the reality as the mult clk is a programmable clock which can take
> > several rates. As we can't trust those values, take them from the clock
> > tree and update the capabilities according to.
> >
> > As we can have the same pitfall, in some cases, with the SAMA5D2 Soc,
> > stop relying on capabilities too.
> >
> > Signed-off-by: Ludovic Desroches
> > ---
> > drivers/mmc/host/sdhci-of-at91.c | 104 +--
> > 1 file changed, 57 insertions(+), 47 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-at91.c
> > b/drivers/mmc/host/sdhci-of-at91.c
> > index e7d1920729fb..a9c126f14d85 100644
> > --- a/drivers/mmc/host/sdhci-of-at91.c
> > +++ b/drivers/mmc/host/sdhci-of-at91.c
> > @@ -30,7 +30,14 @@
> >
> > #define SDHCI_AT91_PRESET_COMMON_CONF 0x400 /* drv type B,
> > programmable clock mode */
> >
> > +struct sdhci_at91_soc_data {
> > + const struct sdhci_pltfm_data *pdata;
> > + bool baseclk_is_generated_internally;
> > + unsigned int divider_for_baseclk;
> > +};
> > +
> > struct sdhci_at91_priv {
> > + const struct sdhci_at91_soc_data *soc_data;
> > struct clk *hclock;
> > struct clk *gck;
> > struct clk *mainck;
> > @@ -130,12 +137,24 @@ static const struct sdhci_ops sdhci_at91_sama5d2_ops
> > = {
> > .set_power = sdhci_at91_set_power,
> > };
> >
> > -static const struct sdhci_pltfm_data soc_data_sama5d2 = {
> > +static const struct sdhci_pltfm_data sdhci_sama5d2_pdata = {
> > .ops = _at91_sama5d2_ops,
> > };
> >
> > +static const struct sdhci_at91_soc_data soc_data_sama5d2 = {
> > + .pdata = _sama5d2_pdata,
> > + .baseclk_is_generated_internally = false,
> > +};
> > +
> > +static const struct sdhci_at91_soc_data soc_data_sam9x60 = {
> > + .pdata = _sama5d2_pdata,
> > + .baseclk_is_generated_internally = true,
> > + .divider_for_baseclk = 2,
> > +};
> > +
> > static const struct of_device_id sdhci_at91_dt_match[] = {
> > { .compatible = "atmel,sama5d2-sdhci", .data = _data_sama5d2 },
> > + { .compatible = "microchip,sam9x60-sdhci", .data = _data_sam9x60 },
> > {}
> > };
> > MODULE_DEVICE_TABLE(of, sdhci_at91_dt_match);
> > @@ -145,50 +164,36 @@ static int sdhci_at91_set_clks_presets(struct device
> > *dev)
> > struct sdhci_host *host = dev_get_drvdata(dev);
> > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host);
> > - int ret;
> > unsigned intcaps0, caps1;
> > unsigned intclk_base, clk_mul;
> > - unsigned intgck_rate, real_gck_rate;
> > + unsigned intgck_rate, clk_base_rate;
> > unsigned intpreset_div;
> >
> > - /*
> > -* The mult clock is provided by as a generated clock by the PMC
> > -* controller. In order to set the rate of gck, we have to get the
> > -* base clock rate and the clock mult from capabilities.
> > -*/
> > clk_prepare_enable(priv->hclock);
> > caps0 = readl(host->ioaddr + SDHCI_CAPABILITIES);
> > caps1 = readl(host->ioaddr + SDHCI_CAPABILITIES_1);
> > - clk_base = (caps0 & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
> > - clk_mul = (caps1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
> > - gck_rate = clk_base * 100 * (clk_mul + 1);
> > - ret = clk_set_rate(priv->gck, gck_rate);
> > - if (ret < 0) {
> > - dev_err(dev, "failed to set gck");
> > - clk_disable_unprepare(priv->hclock);
> > - return ret;
> > - }
> > - /*
> > -* We need to check if we have the requested rate for gck because in
> > -* some cases this rate could be not supported. If it happens, the rate
> > -* is the closest one gck can provide. We have to update the value
> > -* of clk mul.
> > -*/
> > - real_gc
Re: [PATCH] pinctrl: at91-pio4: implement .get_multiple and .set_multiple
On Thu, Sep 05, 2019 at 04:13:04PM +0200, Alexandre Belloni wrote:
>
> Implement .get_multiple and .set_multiple to allow reading or setting
> multiple pins simultaneously. Pins in the same bank will all be switched at
> the same time, improving synchronization and performances.
>
> Signed-off-by: Alexandre Belloni
Acked-by: Ludovic Desroches
Thanks for this improvement. You can keep my ack for v3 as the changes
should be the commit message only. I'll be off for three weeks.
Regards
Ludovic
> ---
> drivers/pinctrl/pinctrl-at91-pio4.c | 60 +
> 1 file changed, 60 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c
> b/drivers/pinctrl/pinctrl-at91-pio4.c
> index d6de4d360cd4..488a302a60d4 100644
> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
> @@ -328,6 +328,35 @@ static int atmel_gpio_get(struct gpio_chip *chip,
> unsigned offset)
> return !!(reg & BIT(pin->line));
> }
>
> +static int atmel_gpio_get_multiple(struct gpio_chip *chip, unsigned long
> *mask,
> +unsigned long *bits)
> +{
> + struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_data(chip);
> + unsigned int bank;
> +
> + bitmap_zero(bits, atmel_pioctrl->npins);
> +
> + for (bank = 0; bank < atmel_pioctrl->nbanks; bank++) {
> + unsigned int word = bank;
> + unsigned int offset = 0;
> + unsigned int reg;
> +
> +#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
> + word = BIT_WORD(bank * ATMEL_PIO_NPINS_PER_BANK);
> + offset = bank * ATMEL_PIO_NPINS_PER_BANK % BITS_PER_LONG;
> +#endif
> + if (!mask[word])
> + continue;
> +
> + reg = atmel_gpio_read(atmel_pioctrl, bank, ATMEL_PIO_PDSR);
> + bits[word] |= mask[word] & (reg << offset);
> +
> + pr_err("ABE: %d %08x\n", bank, bits[word]);
> + }
> +
> + return 0;
> +}
> +
> static int atmel_gpio_direction_output(struct gpio_chip *chip, unsigned
> offset,
> int value)
> {
> @@ -358,11 +387,42 @@ static void atmel_gpio_set(struct gpio_chip *chip,
> unsigned offset, int val)
>BIT(pin->line));
> }
>
> +static void atmel_gpio_set_multiple(struct gpio_chip *chip, unsigned long
> *mask,
> + unsigned long *bits)
> +{
> + struct atmel_pioctrl *atmel_pioctrl = gpiochip_get_data(chip);
> + unsigned int bank;
> +
> + for (bank = 0; bank < atmel_pioctrl->nbanks; bank++) {
> + unsigned int bitmask;
> + unsigned int word = bank;
> +
> +#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
> + word = BIT_WORD(bank * ATMEL_PIO_NPINS_PER_BANK);
> +#endif
> + if (!mask[word])
> + continue;
> +
> + bitmask = mask[word] & bits[word];
> + atmel_gpio_write(atmel_pioctrl, bank, ATMEL_PIO_SODR, bitmask);
> +
> + bitmask = mask[word] & ~bits[word];
> + atmel_gpio_write(atmel_pioctrl, bank, ATMEL_PIO_CODR, bitmask);
> +
> +#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
> + mask[word] >>= ATMEL_PIO_NPINS_PER_BANK;
> + bits[word] >>= ATMEL_PIO_NPINS_PER_BANK;
> +#endif
> + }
> +}
> +
> static struct gpio_chip atmel_gpio_chip = {
> .direction_input= atmel_gpio_direction_input,
> .get= atmel_gpio_get,
> + .get_multiple = atmel_gpio_get_multiple,
> .direction_output = atmel_gpio_direction_output,
> .set= atmel_gpio_set,
> + .set_multiple = atmel_gpio_set_multiple,
> .to_irq = atmel_gpio_to_irq,
> .base = 0,
> };
> --
> 2.21.0
>
>
Re: [PATCH v5 0/9] i2c: add support for filters
On Wed, Sep 11, 2019 at 10:24:14AM +0200, Eugen Hristev - M18282 wrote: > From: Eugen Hristev > > Hello, > > This series adds support for analog and digital filters for i2c controllers > > This series is based on the series: > [PATCH v2 0/9] i2c: at91: filters support for at91 SoCs > and later > [PATCH v4 0/9] i2c: add support for filters > and enhanced to add the bindings for all controllers plus an extra bindings > for the width of the spikes in nanoseconds (digital filters) and cut-off > frequency (analog filters) > > First, bindings are created for > 'i2c-analog-filter' > 'i2c-digital-filter' > 'i2c-digital-filter-width-ns' > 'i2c-analog-filter-cutoff-frequency' > > The support is added in the i2c core to retrieve filter width/cutoff frequency > and add it to the timings structure. > Next, the at91 driver is enhanced for supporting digital filter, advanced > digital filter (with selectable spike width) and the analog filter. > > Finally the device tree for two boards are modified to make use of the > new properties. > > This series is the result of the comments on the ML in the direction > requested: to make the bindings globally available for i2c drivers. > > Changes in v5: > - renamed i2c-filter-width-ns to i2c-digital-filter-width-ns as this > is applicable only to digital filter > - created new binding i2c-digital-filter-width-ns for analog filters. Acked-by: Ludovic Desroches for at91 stuff. You can keep it for the future if needed as long as changes mainly concerned the generic binding. Regards Ludovic > > Changes in v4: > - renamed i2c-ana-filter to i2c-analog-filter > - renamed i2c-dig-filter to i2c-digital-filter > > Changes in v3: > - made bindings global for i2c controllers and modified accordingly > - gave up PADFCDF bit because it's a lack in datasheet > - the computation on the width of the spike is based on periph clock as it > is done for hold time. > > Changes in v2: > - added device tree bindings and support for enable-ana-filt and > enable-dig-filt > - added the new properties to the DT for sama5d4_xplained/sama5d2_xplained > > Eugen Hristev (9): > dt-bindings: i2c: at91: add new compatible > dt-bindings: i2c: add bindings for i2c analog and digital filter > i2c: add support for filters optional properties > i2c: at91: add new platform support for sam9x60 > i2c: at91: add support for digital filtering > i2c: at91: add support for advanced digital filtering > i2c: at91: add support for analog filtering > ARM: dts: at91: sama5d2_xplained: add analog and digital filter for > i2c > ARM: dts: at91: sama5d4_xplained: add digital filter for i2c > > Documentation/devicetree/bindings/i2c/i2c-at91.txt | 3 +- > Documentation/devicetree/bindings/i2c/i2c.txt | 18 > arch/arm/boot/dts/at91-sama5d2_xplained.dts| 6 +++ > arch/arm/boot/dts/at91-sama5d4_xplained.dts| 1 + > drivers/i2c/busses/i2c-at91-core.c | 38 + > drivers/i2c/busses/i2c-at91-master.c | 49 > -- > drivers/i2c/busses/i2c-at91.h | 13 ++ > drivers/i2c/i2c-core-base.c| 6 +++ > include/linux/i2c.h| 6 +++ > 9 files changed, 136 insertions(+), 4 deletions(-) > > -- > 2.7.4 >
Re: [PATCH] i2c: at91: Send bus clear command if SCL or SDA is down
On Wed, Sep 11, 2019 at 12:58:54PM +0300, Codrin Ciubotariu wrote:
> After a transfer timeout, some faulty I2C slave devices might hold down
> the SCL or the SDA pins. We can generate a bus clear command, hoping that
> the slave might release the pins.
>
> Signed-off-by: Codrin Ciubotariu
Acked-by: Ludovic Desroches
I'll be off for three weeks so if there are minor changes, you can keep my
ack.
Thanks
Ludovic
> ---
> drivers/i2c/busses/i2c-at91-master.c | 20
> drivers/i2c/busses/i2c-at91.h| 6 +-
> 2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-at91-master.c
> b/drivers/i2c/busses/i2c-at91-master.c
> index a3fcc35ffd3b..5f544a16db96 100644
> --- a/drivers/i2c/busses/i2c-at91-master.c
> +++ b/drivers/i2c/busses/i2c-at91-master.c
> @@ -599,6 +599,26 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
> at91_twi_write(dev, AT91_TWI_CR,
> AT91_TWI_THRCLR | AT91_TWI_LOCKCLR);
> }
> +
> + /*
> + * After timeout, some faulty I2C slave devices might hold SCL/SDA down;
> + * we can send a bus clear command, hoping that the pins will be
> + * released
> + */
> + if (!(dev->transfer_status & AT91_TWI_SDA) ||
> + !(dev->transfer_status & AT91_TWI_SCL)) {
> + dev_dbg(dev->dev,
> + "SDA/SCL are down; sending bus clear command\n");
> + if (dev->use_alt_cmd) {
> + unsigned int acr;
> +
> + acr = at91_twi_read(dev, AT91_TWI_ACR);
> + acr &= ~AT91_TWI_ACR_DATAL_MASK;
> + at91_twi_write(dev, AT91_TWI_ACR, acr);
> + }
> + at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_CLEAR);
> + }
> +
> return ret;
> }
>
> diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
> index 499b506f6128..ffb870f3ffc6 100644
> --- a/drivers/i2c/busses/i2c-at91.h
> +++ b/drivers/i2c/busses/i2c-at91.h
> @@ -36,6 +36,7 @@
> #define AT91_TWI_SVDIS BIT(5) /* Slave Transfer Disable */
> #define AT91_TWI_QUICK BIT(6) /* SMBus quick command */
> #define AT91_TWI_SWRST BIT(7) /* Software Reset */
> +#define AT91_TWI_CLEAR BIT(15) /* Bus clear command */
> #define AT91_TWI_ACMEN BIT(16) /* Alternative Command Mode
> Enable */
> #define AT91_TWI_ACMDIS BIT(17) /* Alternative Command Mode
> Disable */
> #define AT91_TWI_THRCLR BIT(24) /* Transmit Holding Register
> Clear */
> @@ -69,6 +70,8 @@
> #define AT91_TWI_NACK BIT(8) /* Not Acknowledged */
> #define AT91_TWI_EOSACC BIT(11) /* End Of Slave Access */
> #define AT91_TWI_LOCK BIT(23) /* TWI Lock due to Frame Errors
> */
> +#define AT91_TWI_SCLBIT(24) /* TWI SCL status */
> +#define AT91_TWI_SDABIT(25) /* TWI SDA status */
>
> #define AT91_TWI_INT_MASK \
> (AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY | AT91_TWI_NACK \
> @@ -81,7 +84,8 @@
> #define AT91_TWI_THR0x0034 /* Transmit Holding Register */
>
> #define AT91_TWI_ACR0x0040 /* Alternative Command Register
> */
> -#define AT91_TWI_ACR_DATAL(len) ((len) & 0xff)
> +#define AT91_TWI_ACR_DATAL_MASK GENMASK(15, 0)
> +#define AT91_TWI_ACR_DATAL(len) ((len) & AT91_TWI_ACR_DATAL_MASK)
> #define AT91_TWI_ACR_DIRBIT(8)
>
> #define AT91_TWI_FMR0x0050 /* FIFO Mode Register */
> --
> 2.20.1
>
[PATCH 3/3] ARM: dts: at91: sama5d2: set the sdmmc gclk frequency
Set the frequency of the generated clock used by sdmmc devices in order to not rely on the configuration done by previous components. Signed-off-by: Ludovic Desroches --- arch/arm/boot/dts/sama5d2.dtsi | 4 1 file changed, 4 insertions(+) diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi index 2e2c1a7b1d1d..8d79ff75e3cd 100644 --- a/arch/arm/boot/dts/sama5d2.dtsi +++ b/arch/arm/boot/dts/sama5d2.dtsi @@ -300,6 +300,8 @@ interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>; clocks = < PMC_TYPE_PERIPHERAL 31>, < PMC_TYPE_GCK 31>, < PMC_TYPE_CORE PMC_MAIN>; clock-names = "hclock", "multclk", "baseclk"; + assigned-clocks = < PMC_TYPE_GCK 31>; + assigned-clock-rates = <48000>; status = "disabled"; }; @@ -309,6 +311,8 @@ interrupts = <32 IRQ_TYPE_LEVEL_HIGH 0>; clocks = < PMC_TYPE_PERIPHERAL 32>, < PMC_TYPE_GCK 32>, < PMC_TYPE_CORE PMC_MAIN>; clock-names = "hclock", "multclk", "baseclk"; + assigned-clocks = < PMC_TYPE_GCK 32>; + assigned-clock-rates = <48000>; status = "disabled"; }; -- 2.23.0
[PATCH 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties
There is a new compatible string for the SAM9X60 sdhci device. It involves
an update of the properties about the clocks stuff.
Signed-off-by: Ludovic Desroches
---
.../devicetree/bindings/mmc/sdhci-atmel.txt | 25 ---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
index 1b662d7171a0..364ceea330b6 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
@@ -5,12 +5,19 @@ Documentation/devicetree/bindings/mmc/mmc.txt and the
properties used by the
sdhci-of-at91 driver.
Required properties:
-- compatible: Must be "atmel,sama5d2-sdhci".
+- compatible: Must be "atmel,sama5d2-sdhci" or
"microchip,sam9x60-sdhci".
- clocks: Phandlers to the clocks.
-- clock-names: Must be "hclock", "multclk", "baseclk";
+- clock-names: Must be "hclock", "multclk", "baseclk" for
+ "atmel,sama5d2-sdhci".
+ Must be "hclock", "multclk" for
"microchip,sam9x60-sdhci".
+Optional properties:
+- assigned-clocks: The same with "multclk".
+- assigned-clock-rates The rate of "multclk" in order to not rely on the
+ gck configuration set by previous components.
-Example:
+
+Examples:
sdmmc0: sdio-host@a000 {
compatible = "atmel,sama5d2-sdhci";
@@ -18,4 +25,16 @@ sdmmc0: sdio-host@a000 {
interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>;
clocks = <_hclk>, <_gclk>, <>;
clock-names = "hclock", "multclk", "baseclk";
+ assigned-clocks = <_gclk>;
+ assigned-clock-rates = <48000>;
+};
+
+sdmmc0: sdio-host@8000 {
+ compatible = "microchip,sam9x60-sdhci";
+ reg = <0x8000 0x300>;
+ interrupts = <12 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = < PMC_TYPE_PERIPHERAL 12>, < PMC_TYPE_GCK 12>;
+ clock-names = "hclock", "multclk";
+ assigned-clocks = < PMC_TYPE_GCK 12>;
+ assigned-clock-rates = <1>;
};
--
2.23.0
[PATCH 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device
In the SAM9x60 SoC, there are only two clocks instead of three for the
SDHCI device. The base clk is no longer provided, it is generated
internally from the mult clk.
The values of the base clk and mul in the capabilities registers may not
reflect the reality as the mult clk is a programmable clock which can take
several rates. As we can't trust those values, take them from the clock
tree and update the capabilities according to.
As we can have the same pitfall, in some cases, with the SAMA5D2 Soc,
stop relying on capabilities too.
Signed-off-by: Ludovic Desroches
---
drivers/mmc/host/sdhci-of-at91.c | 104 +--
1 file changed, 57 insertions(+), 47 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index e7d1920729fb..a9c126f14d85 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -30,7 +30,14 @@
#define SDHCI_AT91_PRESET_COMMON_CONF 0x400 /* drv type B, programmable clock
mode */
+struct sdhci_at91_soc_data {
+ const struct sdhci_pltfm_data *pdata;
+ bool baseclk_is_generated_internally;
+ unsigned int divider_for_baseclk;
+};
+
struct sdhci_at91_priv {
+ const struct sdhci_at91_soc_data *soc_data;
struct clk *hclock;
struct clk *gck;
struct clk *mainck;
@@ -130,12 +137,24 @@ static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
.set_power = sdhci_at91_set_power,
};
-static const struct sdhci_pltfm_data soc_data_sama5d2 = {
+static const struct sdhci_pltfm_data sdhci_sama5d2_pdata = {
.ops = _at91_sama5d2_ops,
};
+static const struct sdhci_at91_soc_data soc_data_sama5d2 = {
+ .pdata = _sama5d2_pdata,
+ .baseclk_is_generated_internally = false,
+};
+
+static const struct sdhci_at91_soc_data soc_data_sam9x60 = {
+ .pdata = _sama5d2_pdata,
+ .baseclk_is_generated_internally = true,
+ .divider_for_baseclk = 2,
+};
+
static const struct of_device_id sdhci_at91_dt_match[] = {
{ .compatible = "atmel,sama5d2-sdhci", .data = _data_sama5d2 },
+ { .compatible = "microchip,sam9x60-sdhci", .data = _data_sam9x60 },
{}
};
MODULE_DEVICE_TABLE(of, sdhci_at91_dt_match);
@@ -145,50 +164,36 @@ static int sdhci_at91_set_clks_presets(struct device *dev)
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host);
- int ret;
unsigned intcaps0, caps1;
unsigned intclk_base, clk_mul;
- unsigned intgck_rate, real_gck_rate;
+ unsigned intgck_rate, clk_base_rate;
unsigned intpreset_div;
- /*
-* The mult clock is provided by as a generated clock by the PMC
-* controller. In order to set the rate of gck, we have to get the
-* base clock rate and the clock mult from capabilities.
-*/
clk_prepare_enable(priv->hclock);
caps0 = readl(host->ioaddr + SDHCI_CAPABILITIES);
caps1 = readl(host->ioaddr + SDHCI_CAPABILITIES_1);
- clk_base = (caps0 & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
- clk_mul = (caps1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
- gck_rate = clk_base * 100 * (clk_mul + 1);
- ret = clk_set_rate(priv->gck, gck_rate);
- if (ret < 0) {
- dev_err(dev, "failed to set gck");
- clk_disable_unprepare(priv->hclock);
- return ret;
- }
- /*
-* We need to check if we have the requested rate for gck because in
-* some cases this rate could be not supported. If it happens, the rate
-* is the closest one gck can provide. We have to update the value
-* of clk mul.
-*/
- real_gck_rate = clk_get_rate(priv->gck);
- if (real_gck_rate != gck_rate) {
- clk_mul = real_gck_rate / (clk_base * 100) - 1;
- caps1 &= (~SDHCI_CLOCK_MUL_MASK);
- caps1 |= ((clk_mul << SDHCI_CLOCK_MUL_SHIFT) &
- SDHCI_CLOCK_MUL_MASK);
- /* Set capabilities in r/w mode. */
- writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN,
- host->ioaddr + SDMMC_CACR);
- writel(caps1, host->ioaddr + SDHCI_CAPABILITIES_1);
- /* Set capabilities in ro mode. */
- writel(0, host->ioaddr + SDMMC_CACR);
- dev_info(dev, "update clk mul to %u as gck rate is %u Hz\n",
-clk_mul, real_gck_rate);
- }
+
+ gck_rate = clk_get_rate(priv->gck);
+ if (priv->soc_data->baseclk_is_generated_internally)
+ clk_base_rate = gck_r
Re: [PATCH 2/2] ARM: dts: at91: sama5d27_som1_ek: add mmc capabilities for SDMMC0
On Mon, Aug 12, 2019 at 03:38:34PM +, eugen.hris...@microchip.com wrote: > On 09.08.2019 09:23, Ludovic Desroches wrote: > > On Thu, Aug 08, 2019 at 03:57:30PM +0300, Adrian Hunter wrote: > >> On 8/08/19 3:42 PM, Ludovic Desroches wrote: > >>> On Thu, Aug 08, 2019 at 10:35:43AM +0200, Eugen Hristev - M18282 wrote: > >>>> From: Eugen Hristev > >>>> > >>>> Add mmc capabilities for SDMMC0 for this board. > >>>> With this enabled, eMMC connected card is detected as: > >>>> > >>>> mmc0: new DDR MMC card at address 0001 > >>>> > >>>> Signed-off-by: Eugen Hristev > >>> Acked-by: Ludovic Desroches > >>> > >>> I am interested to have the some insights about the use of sd-uhs-* > >>> properties. > >>> > >>> Our IP can't deal with 1V8 by itself. It has a 1V8SEL signal which can > >>> be used as the logic control input of a mux. So even if the IP claims > >>> to support UHS modes, it depends on the board. > >>> > >>> Are the sd-uhs-* properties a way to deal with this? I tend to think no > >>> as sdhci_setup_host() will set the caps depending on the content of the > >>> capabilities register. Do we have to use the SDHCI_QUIRK_MISSING_CAPS > >>> quirk or sdhci-caps/sdhci-caps-mask? > >> > >> There is "no-1-8-v" which it looks like sdhci-of-at91.c already supports: > >> > >>sdhci_at91_probe() -> sdhci_get_of_property() -> sdhci_get_property() > >> > >>if (device_property_present(dev, "no-1-8-v")) > >>host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V; > >> > > > > Right, I forgot this property. Thanks. > > > > Eugen, do you see cases we can't cover with this property? > > Hi, > > For current requirements and driver support, this should be enough. > > I noticed one thing regarding SD-Cards, if I add property sd-uhs-sdr104 > the class 10 uhs1 cards are detected as SDR104 . Without this property > they are detected as DDR50. Any idea why the difference ? The controller > does not claim to have SDR104 support ? We should add it ? With the mainline, our tree or both? In our tree, SDR104 is removed from the capabilities. Ludovic
Re: [PATCH 1/2] mmc: sdhci-of-at91: add quirk for broken HS200
On Thu, Aug 08, 2019 at 05:23:00PM +0200, Ulf Hansson wrote:
> On Thu, 8 Aug 2019 at 10:35, wrote:
> >
> > From: Eugen Hristev
> >
> > HS200 is not implemented in the driver, but the controller claims it
> > through caps.
> > Remove it via quirk.
> > Without this quirk, the mmc core will try to enable hs200, which will fail,
> > and the eMMC initialization will fail.
> >
> > Signed-off-by: Eugen Hristev
>
> Should this be applied as a fix and possibly tagged for stable?
>
> In such case, do you have a specific commit that it fixes?
I think so, I would say:
Fixes: bb5f8ea4d514 ("mmc: sdhci-of-at91: introduce driver for the Atmel SDMMC")
Cc: sta...@vger.kernel.org #v4.4 and later
It doesn't apply on 4.4 but resolution is trivial.
Regards
Ludovic
>
> Kind regards
> Uffe
>
> > ---
> > drivers/mmc/host/sdhci-of-at91.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-at91.c
> > b/drivers/mmc/host/sdhci-of-at91.c
> > index 57fe3b2..3a8c6d8 100644
> > --- a/drivers/mmc/host/sdhci-of-at91.c
> > +++ b/drivers/mmc/host/sdhci-of-at91.c
> > @@ -370,6 +370,9 @@ static int sdhci_at91_probe(struct platform_device
> > *pdev)
> > pm_runtime_set_autosuspend_delay(>dev, 50);
> > pm_runtime_use_autosuspend(>dev);
> >
> > + /* HS200 is broken at this moment */
> > + host->quirks2 = SDHCI_QUIRK2_BROKEN_HS200;
> > +
> > ret = sdhci_add_host(host);
> > if (ret)
> > goto pm_runtime_disable;
> > --
> > 2.7.4
> >
Re: [PATCH 2/2] ARM: dts: at91: sama5d27_som1_ek: add mmc capabilities for SDMMC0
On Thu, Aug 08, 2019 at 03:57:30PM +0300, Adrian Hunter wrote:
> On 8/08/19 3:42 PM, Ludovic Desroches wrote:
> > On Thu, Aug 08, 2019 at 10:35:43AM +0200, Eugen Hristev - M18282 wrote:
> >> From: Eugen Hristev
> >>
> >> Add mmc capabilities for SDMMC0 for this board.
> >> With this enabled, eMMC connected card is detected as:
> >>
> >> mmc0: new DDR MMC card at address 0001
> >>
> >> Signed-off-by: Eugen Hristev
> > Acked-by: Ludovic Desroches
> >
> > I am interested to have the some insights about the use of sd-uhs-*
> > properties.
> >
> > Our IP can't deal with 1V8 by itself. It has a 1V8SEL signal which can
> > be used as the logic control input of a mux. So even if the IP claims
> > to support UHS modes, it depends on the board.
> >
> > Are the sd-uhs-* properties a way to deal with this? I tend to think no
> > as sdhci_setup_host() will set the caps depending on the content of the
> > capabilities register. Do we have to use the SDHCI_QUIRK_MISSING_CAPS
> > quirk or sdhci-caps/sdhci-caps-mask?
>
> There is "no-1-8-v" which it looks like sdhci-of-at91.c already supports:
>
> sdhci_at91_probe() -> sdhci_get_of_property() -> sdhci_get_property()
>
> if (device_property_present(dev, "no-1-8-v"))
> host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
>
Right, I forgot this property. Thanks.
Eugen, do you see cases we can't cover with this property?
Regards
Ludovic
>
> >
> > Regards
> >
> > Ludovic
> >
> >> ---
> >> arch/arm/boot/dts/at91-sama5d27_som1_ek.dts | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> >> b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> >> index 149e539..194b3a3 100644
> >> --- a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> >> +++ b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> >> @@ -54,6 +54,7 @@
> >>
> >>sdmmc0: sdio-host@a000 {
> >>bus-width = <8>;
> >> + mmc-ddr-3_3v;
> >>pinctrl-names = "default";
> >>pinctrl-0 = <_sdmmc0_default>;
> >>status = "okay";
> >> --
> >> 2.7.4
> >>
> >
>
Re: [PATCH 1/2] mmc: sdhci-of-at91: add quirk for broken HS200
On Thu, Aug 08, 2019 at 10:35:40AM +0200, Eugen Hristev - M18282 wrote: > From: Eugen Hristev > > HS200 is not implemented in the driver, but the controller claims it > through caps. > Remove it via quirk. > Without this quirk, the mmc core will try to enable hs200, which will fail, > and the eMMC initialization will fail. > > Signed-off-by: Eugen Hristev Acked-by: Ludovic Desroches Thanks Ludovic > --- > drivers/mmc/host/sdhci-of-at91.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-of-at91.c > b/drivers/mmc/host/sdhci-of-at91.c > index 57fe3b2..3a8c6d8 100644 > --- a/drivers/mmc/host/sdhci-of-at91.c > +++ b/drivers/mmc/host/sdhci-of-at91.c > @@ -370,6 +370,9 @@ static int sdhci_at91_probe(struct platform_device *pdev) > pm_runtime_set_autosuspend_delay(>dev, 50); > pm_runtime_use_autosuspend(>dev); > > + /* HS200 is broken at this moment */ > + host->quirks2 = SDHCI_QUIRK2_BROKEN_HS200; > + > ret = sdhci_add_host(host); > if (ret) > goto pm_runtime_disable; > -- > 2.7.4 >
Re: [PATCH 2/2] ARM: dts: at91: sama5d27_som1_ek: add mmc capabilities for SDMMC0
On Thu, Aug 08, 2019 at 10:35:43AM +0200, Eugen Hristev - M18282 wrote:
> From: Eugen Hristev
>
> Add mmc capabilities for SDMMC0 for this board.
> With this enabled, eMMC connected card is detected as:
>
> mmc0: new DDR MMC card at address 0001
>
> Signed-off-by: Eugen Hristev
Acked-by: Ludovic Desroches
I am interested to have the some insights about the use of sd-uhs-*
properties.
Our IP can't deal with 1V8 by itself. It has a 1V8SEL signal which can
be used as the logic control input of a mux. So even if the IP claims
to support UHS modes, it depends on the board.
Are the sd-uhs-* properties a way to deal with this? I tend to think no
as sdhci_setup_host() will set the caps depending on the content of the
capabilities register. Do we have to use the SDHCI_QUIRK_MISSING_CAPS
quirk or sdhci-caps/sdhci-caps-mask?
Regards
Ludovic
> ---
> arch/arm/boot/dts/at91-sama5d27_som1_ek.dts | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> index 149e539..194b3a3 100644
> --- a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> +++ b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
> @@ -54,6 +54,7 @@
>
> sdmmc0: sdio-host@a000 {
> bus-width = <8>;
> + mmc-ddr-3_3v;
> pinctrl-names = "default";
> pinctrl-0 = <_sdmmc0_default>;
> status = "okay";
> --
> 2.7.4
>
Re: [PATCH] mmc: atmel-mci: Mark expected switch fall-throughs
On Wed, Jul 31, 2019 at 09:06:07AM -0700, Kees Cook wrote: > On Wed, Jul 31, 2019 at 01:32:16PM +0200, Ludovic Desroches wrote: > > > drivers/mmc/host/atmel-mci.c:2426:40: warning: this statement may fall > > > through [-Wimplicit-fallthrough=] > > >host->caps.need_notbusy_for_read_ops = 1; > > >~^~~ > > > drivers/mmc/host/atmel-mci.c:2427:2: note: here > > > case 0x100: > > > ^~~~ > > > > > > Reported-by: Stephen Rothwell > > > Signed-off-by: Gustavo A. R. Silva > > > > I don't know if there is a policy in the kernel about the expression to > > use. As this one does the job > > Yup, documented here: > https://www.kernel.org/doc/html/latest/process/deprecated.html#implicit-switch-case-fall-through Thanks for the pointer. Regards Ludovic > > > Acked-by: Ludovic Desroches > > Thanks! > > -- > Kees Cook
Re: [PATCH] crypto: atmel-sha204a: Use device-managed registration API
On Tue, Jul 23, 2019 at 03:19:36PM +0800, Chuhong Yuan wrote: > > Use devm_hwrng_register to get rid of manual > unregistration. > > Signed-off-by: Chuhong Yuan Acked-by: Ludovic Desroches Thanks > --- > drivers/crypto/atmel-sha204a.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c > index ea0d2068ea4f..c96c14e7dab1 100644 > --- a/drivers/crypto/atmel-sha204a.c > +++ b/drivers/crypto/atmel-sha204a.c > @@ -109,7 +109,7 @@ static int atmel_sha204a_probe(struct i2c_client *client, > i2c_priv->hwrng.read = atmel_sha204a_rng_read; > i2c_priv->hwrng.quality = 1024; > > - ret = hwrng_register(_priv->hwrng); > + ret = devm_hwrng_register(>dev, _priv->hwrng); > if (ret) > dev_warn(>dev, "failed to register RNG (%d)\n", ret); > > @@ -127,7 +127,6 @@ static int atmel_sha204a_remove(struct i2c_client *client) > > if (i2c_priv->hwrng.priv) > kfree((void *)i2c_priv->hwrng.priv); > - hwrng_unregister(_priv->hwrng); > > return 0; > } > -- > 2.20.1 > >
Re: [PATCH] mmc: atmel-mci: Mark expected switch fall-throughs
Hi, On Sun, Jul 28, 2019 at 07:01:23PM -0500, Gustavo A. R. Silva wrote: > > Mark switch cases where we are expecting to fall through. > > This patch fixes the following warnings: > > drivers/mmc/host/atmel-mci.c: In function 'atmci_get_cap': > drivers/mmc/host/atmel-mci.c:2415:30: warning: this statement may fall > through [-Wimplicit-fallthrough=] >host->caps.has_odd_clk_div = 1; >~~~^~~ > drivers/mmc/host/atmel-mci.c:2416:2: note: here > case 0x400: > ^~~~ > drivers/mmc/host/atmel-mci.c:2422:28: warning: this statement may fall > through [-Wimplicit-fallthrough=] >host->caps.has_highspeed = 1; >~^~~ > drivers/mmc/host/atmel-mci.c:2423:2: note: here > case 0x200: > ^~~~ > drivers/mmc/host/atmel-mci.c:2426:40: warning: this statement may fall > through [-Wimplicit-fallthrough=] >host->caps.need_notbusy_for_read_ops = 1; >~^~~ > drivers/mmc/host/atmel-mci.c:2427:2: note: here > case 0x100: > ^~~~ > > Reported-by: Stephen Rothwell > Signed-off-by: Gustavo A. R. Silva I don't know if there is a policy in the kernel about the expression to use. As this one does the job Acked-by: Ludovic Desroches Thanks > --- > drivers/mmc/host/atmel-mci.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c > index 9ee0bc0ce6d0..c26fbe5f 100644 > --- a/drivers/mmc/host/atmel-mci.c > +++ b/drivers/mmc/host/atmel-mci.c > @@ -2413,6 +2413,7 @@ static void atmci_get_cap(struct atmel_mci *host) > case 0x600: > case 0x500: > host->caps.has_odd_clk_div = 1; > + /* Fall through */ > case 0x400: > case 0x300: > host->caps.has_dma_conf_reg = 1; > @@ -2420,13 +2421,16 @@ static void atmci_get_cap(struct atmel_mci *host) > host->caps.has_cfg_reg = 1; > host->caps.has_cstor_reg = 1; > host->caps.has_highspeed = 1; > + /* Fall through */ > case 0x200: > host->caps.has_rwproof = 1; > host->caps.need_blksz_mul_4 = 0; > host->caps.need_notbusy_for_read_ops = 1; > + /* Fall through */ > case 0x100: > host->caps.has_bad_data_ordering = 0; > host->caps.need_reset_after_xfer = 0; > + /* Fall through */ > case 0x0: > break; > default: > -- > 2.22.0 > >
Re: [PATCH v3 0/9] i2c: add support for filters
On Tue, Jul 09, 2019 at 03:19:26PM +0200, Eugen Hristev - M18282 wrote: > From: Eugen Hristev > > Hello, > > This series adds support for analog and digital filters for i2c controllers > > This series is based on the series: > [PATCH v2 0/9] i2c: at91: filters support for at91 SoCs > and enhanced to add the bindings for all controllers plus an extra binding > for the width of the spikes in nanoseconds. > > First, bindings are created for > 'i2c-ana-filter' > 'i2c-dig-filter' > 'i2c-filter-width-ns' > > The support is added in the i2c core to retrieve filter width and add it > to the timings structure. > Next, the at91 driver is enhanced for supporting digital filter, advanced > digital filter (with selectable spike width) and the analog filter. > > Finally the device tree for two boards are modified to make use of the > new properties. > > This series is the result of the comments on the ML in the direction > requested: to make the bindings globally available for i2c drivers. > > Changes in v3: > - made bindings global for i2c controllers and modified accordingly > - gave up PADFCDF bit because it's a lack in datasheet > - the computation on the width of the spike is based on periph clock as it > is done for hold time. > > Changes in v2: > - added device tree bindings and support for enable-ana-filt and > enable-dig-filt > - added the new properties to the DT for sama5d4_xplained/sama5d2_xplained > > Eugen Hristev (9): > dt-bindings: i2c: at91: add new compatible > dt-bindings: i2c: add bindings for i2c analog and digital filter > i2c: add support for filter-width-ns optional property > i2c: at91: add new platform support for sam9x60 > i2c: at91: add support for digital filtering > i2c: at91: add support for advanced digital filtering > i2c: at91: add support for analog filtering > ARM: dts: at91: sama5d2_xplained: add analog and digital filter for > i2c > ARM: dts: at91: sama5d4_xplained: add analog filter for i2c > > Documentation/devicetree/bindings/i2c/i2c-at91.txt | 3 +- > Documentation/devicetree/bindings/i2c/i2c.txt | 11 + > arch/arm/boot/dts/at91-sama5d2_xplained.dts| 6 +++ > arch/arm/boot/dts/at91-sama5d4_xplained.dts| 1 + > drivers/i2c/busses/i2c-at91-core.c | 38 + > drivers/i2c/busses/i2c-at91-master.c | 49 > -- > drivers/i2c/busses/i2c-at91.h | 13 ++ > drivers/i2c/i2c-core-base.c| 2 + > include/linux/i2c.h| 2 + > 9 files changed, 121 insertions(+), 4 deletions(-) Hi, I don't know if it will fit other vendors need concerning the binding but for Microchip it sounds good. Acked-by: Ludovic Desroches for the whole serie. Regards Ludovic
Re: [PATCH] dmaengine: at_xdmac: check for non-empty xfers_list before invoking callback
On Sat, Jun 29, 2019 at 01:50:48PM +0530, Raag Jadav wrote:
>
> tx descriptor retrieved from an empty xfers_list may not have valid
> pointers to the callback functions.
> Avoid calling dmaengine_desc_get_callback_invoke if xfers_list is empty.
>
> Signed-off-by: Raag Jadav
Acked-by: Ludovic Desroches
Thanks
> ---
> drivers/dma/at_xdmac.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index 627ef3e..b58ac72 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -1568,11 +1568,14 @@ static void at_xdmac_handle_cyclic(struct
> at_xdmac_chan *atchan)
> struct at_xdmac_desc*desc;
> struct dma_async_tx_descriptor *txd;
>
> - desc = list_first_entry(>xfers_list, struct at_xdmac_desc,
> xfer_node);
> - txd = >tx_dma_desc;
> + if (!list_empty(>xfers_list)) {
> + desc = list_first_entry(>xfers_list,
> + struct at_xdmac_desc, xfer_node);
> + txd = >tx_dma_desc;
>
> - if (txd->flags & DMA_PREP_INTERRUPT)
> - dmaengine_desc_get_callback_invoke(txd, NULL);
> + if (txd->flags & DMA_PREP_INTERRUPT)
> + dmaengine_desc_get_callback_invoke(txd, NULL);
> + }
> }
>
> static void at_xdmac_handle_error(struct at_xdmac_chan *atchan)
> --
> 2.7.4
>
>
Re: [PATCH] clk: at91: generated: Truncate divisor to GENERATED_MAX_DIV + 1
On Wed, Jun 12, 2019 at 03:54:00PM +0200, Nicolas Ferre - M43238 wrote:
> On 10/06/2019 at 17:20, Codrin Ciubotariu - M19940 wrote:
> > From: Codrin Ciubotariu
> >
> > In clk_generated_determine_rate(), if the divisor is greater than
> > GENERATED_MAX_DIV + 1, then the wrong best_rate will be returned.
> > If clk_generated_set_rate() will be called later with this wrong
> > rate, it will return -EINVAL, so the generated clock won't change
> > its value. Do no let the divisor be greater than GENERATED_MAX_DIV + 1.
> >
> > Fixes: 8c7aa6328947 ("clk: at91: clk-generated: remove useless divisor
> > loop")
> > Signed-off-by: Codrin Ciubotariu
>
> Yes:
> Acked-by: Nicolas Ferre
Acked-by: Ludovic Desroches
Thanks
Ludovic
>
> Thanks for having fixed this Codrin. Best regards,
>Nicolas
>
> > ---
> > drivers/clk/at91/clk-generated.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/clk/at91/clk-generated.c
> > b/drivers/clk/at91/clk-generated.c
> > index 5f18847965c1..290cffe35deb 100644
> > --- a/drivers/clk/at91/clk-generated.c
> > +++ b/drivers/clk/at91/clk-generated.c
> > @@ -146,6 +146,8 @@ static int clk_generated_determine_rate(struct clk_hw
> > *hw,
> > continue;
> >
> > div = DIV_ROUND_CLOSEST(parent_rate, req->rate);
> > + if (div > GENERATED_MAX_DIV + 1)
> > + div = GENERATED_MAX_DIV + 1;
> >
> > clk_generated_best_diff(req, parent, parent_rate, div,
> > _diff, _rate);
> >
>
>
> --
> Nicolas Ferre
Re: [PATCH] i2c: at91: handle TXRDY interrupt spam
On Mon, May 06, 2019 at 10:19:01AM +0200, Eugen Hristev - M18282 wrote:
>
>
> On 04.05.2019 02:58, Raag Jadav wrote:
>
> > On Thu, May 02, 2019 at 04:01:16PM +0200, Ludovic Desroches wrote:
> >> On Tue, Apr 30, 2019 at 04:03:32AM +0530, Raag Jadav wrote:
> >>> External E-Mail
> >>>
> >>>
> >>> On Mon, Apr 29, 2019 at 11:00:05AM +0200, Ludovic Desroches wrote:
> >>>> Hello Raag,
> >>>>
> >>>> On Tue, Apr 23, 2019 at 01:06:48PM +0530, Raag Jadav wrote:
> >>>>> External E-Mail
> >>>>>
> >>>>>
> >>>>> Performing i2c write operation while SDA or SCL line is held
> >>>>> or grounded by slave device, we go into infinite
> >>>>> at91_twi_write_next_byte
> >>>>> loop with TXRDY interrupt spam.
> >>>>
> >>>> Sorry but I am not sure to have the full picture, the controller is in
> >>>> slave or master mode?
> >>>>
> >>>> SVREAD is only used in slave mode. When SVREAD is set, it means that a
> >>>> read
> >>>> access is performed and your issue concerns the write operation.
> >>>>
> >>>> Regards
> >>>>
> >>>> Ludovic
> >>>
> >>> Yes, even though the datasheet suggests that SVREAD is irrelevant in
> >>> master mode,
> >>> TXRDY and SVREAD are the only ones being set in status register upon
> >>> reproducing the issue.
> >>> Couldn't think of a better way to handle such strange behaviour.
> >>> Any suggestions would be appreciated.
> >>
> >> I have the confirmation that you can't rely on the SVREAD flag when in
> >> master mode. This flag should always have the same value.
> >>
> >> I am trying to understand what could lead to your situation. Can you
> >> give me more details. What kind of device it is? What does lead to this
> >> situation? Does it happen randomly or not?
> >
> > One of the sama5d2 based board I worked on, was having trouble complete its
> > boot
> > because of a faulty i2c device, which was randomly holding down the SDA line
> > on i2c write operation, not allowing the controller to complete its
> > transmission,
> > causing a massive TXRDY interrupt spam, ultimately hanging the processor.
> >
> > Another strange observation was that SVREAD was being set in the status
> > register
> > along with TXRDY, every time I reproduced the issue.
> > You can reproduce it by simply grounding the SDA line and performing i2c
> > write
> > on the bus.
> >
> > Note that NACK, LOCK or TXCOMP are never set as the transmission never
> > completes.
> > I'm not sure why slave bits are being set in master mode,
> > but it's been working reliably for me.
> >
> > This patch doesn't recover the SDA line. It just prevents the processor from
> > getting hanged in case of i2c bus lockup.
>
> Hello,
>
> I have noticed the same hanging at some points... In my case it is
> because of this patch:
>
> commit e8f39e9fc0e0b7bce24922da925af820bacb8ef8
> Author: David Engraf
> Date: Thu Apr 26 11:53:14 2018 +0200
>
Good to know.
>
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> index bfd1fdf..3f3e8b3 100644
> --- a/drivers/i2c/busses/i2c-at91.c
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -518,8 +518,16 @@ static irqreturn_t atmel_twi_interrupt(int irq,
> void *dev_id)
> * the RXRDY interrupt first in order to not keep garbage data
> in the
> * Receive Holding Register for the next transfer.
> */
> - if (irqstatus & AT91_TWI_RXRDY)
> - at91_twi_read_next_byte(dev);
> + if (irqstatus & AT91_TWI_RXRDY) {
> + /*
> +* Read all available bytes at once by polling RXRDY
> usable w/
> +* and w/o FIFO. With FIFO enabled we could also read
> RXFL and
> +* avoid polling RXRDY.
> +*/
> + do {
> + at91_twi_read_next_byte(dev);
> + } while (at91_twi_read(dev, AT91_TWI_SR) & AT91_TWI_RXRDY);
> + }
>
>
> In my opinion having a do/while with an exit condition relying solely on
> a bit read from hardware is unacceptable in IRQ context - kernel can
> hang here.
> A timeout would be a solution...
You're right with a faulty hardware it can lead to disaster. As you
mentionned issues with this
Re: [PATCH] i2c: at91: handle TXRDY interrupt spam
On Sat, May 04, 2019 at 05:28:51AM +0530, Raag Jadav wrote:
> On Thu, May 02, 2019 at 04:01:16PM +0200, Ludovic Desroches wrote:
> > On Tue, Apr 30, 2019 at 04:03:32AM +0530, Raag Jadav wrote:
> > > External E-Mail
> > >
> > >
> > > On Mon, Apr 29, 2019 at 11:00:05AM +0200, Ludovic Desroches wrote:
> > > > Hello Raag,
> > > >
> > > > On Tue, Apr 23, 2019 at 01:06:48PM +0530, Raag Jadav wrote:
> > > > > External E-Mail
> > > > >
> > > > >
> > > > > Performing i2c write operation while SDA or SCL line is held
> > > > > or grounded by slave device, we go into infinite
> > > > > at91_twi_write_next_byte
> > > > > loop with TXRDY interrupt spam.
> > > >
> > > > Sorry but I am not sure to have the full picture, the controller is in
> > > > slave or master mode?
> > > >
> > > > SVREAD is only used in slave mode. When SVREAD is set, it means that a
> > > > read
> > > > access is performed and your issue concerns the write operation.
> > > >
> > > > Regards
> > > >
> > > > Ludovic
> > >
> > > Yes, even though the datasheet suggests that SVREAD is irrelevant in
> > > master mode,
> > > TXRDY and SVREAD are the only ones being set in status register upon
> > > reproducing the issue.
> > > Couldn't think of a better way to handle such strange behaviour.
> > > Any suggestions would be appreciated.
> >
> > I have the confirmation that you can't rely on the SVREAD flag when in
> > master mode. This flag should always have the same value.
> >
> > I am trying to understand what could lead to your situation. Can you
> > give me more details. What kind of device it is? What does lead to this
> > situation? Does it happen randomly or not?
>
> One of the sama5d2 based board I worked on, was having trouble complete its
> boot
> because of a faulty i2c device, which was randomly holding down the SDA line
> on i2c write operation, not allowing the controller to complete its
> transmission,
> causing a massive TXRDY interrupt spam, ultimately hanging the processor.
>
> Another strange observation was that SVREAD was being set in the status
> register
> along with TXRDY, every time I reproduced the issue.
> You can reproduce it by simply grounding the SDA line and performing i2c write
> on the bus.
Thanks for the details, I'll discussed it with hw guys but expect some
dealy as I'll be off next 2 weeks.
Regards
Ludovic
>
> Note that NACK, LOCK or TXCOMP are never set as the transmission never
> completes.
> I'm not sure why slave bits are being set in master mode,
> but it's been working reliably for me.
>
> This patch doesn't recover the SDA line. It just prevents the processor from
> getting hanged in case of i2c bus lockup.
>
> Cheers,
> Raag
>
> >
> > Regards
> >
> > Ludovic
> >
> > >
> > > Cheers,
> > > Raag
> > >
> > > >
> > > > >
> > > > > Signed-off-by: Raag Jadav
> > > > > ---
> > > > > drivers/i2c/busses/i2c-at91.c | 6 +-
> > > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/i2c/busses/i2c-at91.c
> > > > > b/drivers/i2c/busses/i2c-at91.c
> > > > > index 3f3e8b3..b2f5fdb 100644
> > > > > --- a/drivers/i2c/busses/i2c-at91.c
> > > > > +++ b/drivers/i2c/busses/i2c-at91.c
> > > > > @@ -72,6 +72,7 @@
> > > > > #define AT91_TWI_TXCOMP BIT(0) /* Transmission
> > > > > Complete */
> > > > > #define AT91_TWI_RXRDY BIT(1) /* Receive Holding
> > > > > Register Ready */
> > > > > #define AT91_TWI_TXRDY BIT(2) /* Transmit Holding
> > > > > Register Ready */
> > > > > +#define AT91_TWI_SVREAD BIT(3) /* Slave Read */
> > > > > #define AT91_TWI_OVRE BIT(6) /* Overrun Error */
> > > > > #define AT91_TWI_UNRE BIT(7) /* Underrun Error */
> > > > > #define AT91_TWI_NACK BIT(8) /* Not Acknowledged */
> > > > > @@ -571,7 +572,10 @@ static irqreturn_t atmel_twi_interrupt(int irq,
> > > > > void *dev_id)
> > > > > at91_disable_twi_interrupts(dev);
> > > > > complete(>cmd_complete);
> > > > > } else if (irqstatus & AT91_TWI_TXRDY) {
> > > > > - at91_twi_write_next_byte(dev);
> > > > > + if ((status & AT91_TWI_SVREAD) && (dev->buf_len == 0))
> > > > > + at91_twi_write(dev, AT91_TWI_IDR,
> > > > > AT91_TWI_TXRDY);
> > > > > + else
> > > > > + at91_twi_write_next_byte(dev);
> > > > > }
> > > > >
> > > > > /* catch error flags */
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > >
> > >
> > > ___
> > > linux-arm-kernel mailing list
> > > linux-arm-ker...@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > >
Re: [PATCH] i2c: at91: handle TXRDY interrupt spam
On Tue, Apr 30, 2019 at 04:03:32AM +0530, Raag Jadav wrote:
> External E-Mail
>
>
> On Mon, Apr 29, 2019 at 11:00:05AM +0200, Ludovic Desroches wrote:
> > Hello Raag,
> >
> > On Tue, Apr 23, 2019 at 01:06:48PM +0530, Raag Jadav wrote:
> > > External E-Mail
> > >
> > >
> > > Performing i2c write operation while SDA or SCL line is held
> > > or grounded by slave device, we go into infinite at91_twi_write_next_byte
> > > loop with TXRDY interrupt spam.
> >
> > Sorry but I am not sure to have the full picture, the controller is in
> > slave or master mode?
> >
> > SVREAD is only used in slave mode. When SVREAD is set, it means that a read
> > access is performed and your issue concerns the write operation.
> >
> > Regards
> >
> > Ludovic
>
> Yes, even though the datasheet suggests that SVREAD is irrelevant in master
> mode,
> TXRDY and SVREAD are the only ones being set in status register upon
> reproducing the issue.
> Couldn't think of a better way to handle such strange behaviour.
> Any suggestions would be appreciated.
I have the confirmation that you can't rely on the SVREAD flag when in
master mode. This flag should always have the same value.
I am trying to understand what could lead to your situation. Can you
give me more details. What kind of device it is? What does lead to this
situation? Does it happen randomly or not?
Regards
Ludovic
>
> Cheers,
> Raag
>
> >
> > >
> > > Signed-off-by: Raag Jadav
> > > ---
> > > drivers/i2c/busses/i2c-at91.c | 6 +-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> > > index 3f3e8b3..b2f5fdb 100644
> > > --- a/drivers/i2c/busses/i2c-at91.c
> > > +++ b/drivers/i2c/busses/i2c-at91.c
> > > @@ -72,6 +72,7 @@
> > > #define AT91_TWI_TXCOMP BIT(0) /* Transmission Complete */
> > > #define AT91_TWI_RXRDY BIT(1) /* Receive Holding Register
> > > Ready */
> > > #define AT91_TWI_TXRDY BIT(2) /* Transmit Holding Register
> > > Ready */
> > > +#define AT91_TWI_SVREAD BIT(3) /* Slave Read */
> > > #define AT91_TWI_OVRE BIT(6) /* Overrun Error */
> > > #define AT91_TWI_UNRE BIT(7) /* Underrun Error */
> > > #define AT91_TWI_NACK BIT(8) /* Not Acknowledged */
> > > @@ -571,7 +572,10 @@ static irqreturn_t atmel_twi_interrupt(int irq, void
> > > *dev_id)
> > > at91_disable_twi_interrupts(dev);
> > > complete(>cmd_complete);
> > > } else if (irqstatus & AT91_TWI_TXRDY) {
> > > - at91_twi_write_next_byte(dev);
> > > + if ((status & AT91_TWI_SVREAD) && (dev->buf_len == 0))
> > > + at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_TXRDY);
> > > + else
> > > + at91_twi_write_next_byte(dev);
> > > }
> > >
> > > /* catch error flags */
> > > --
> > > 2.7.4
> > >
> > >
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Re: [PATCH] i2c: at91: handle TXRDY interrupt spam
Hello Raag,
On Tue, Apr 23, 2019 at 01:06:48PM +0530, Raag Jadav wrote:
> External E-Mail
>
>
> Performing i2c write operation while SDA or SCL line is held
> or grounded by slave device, we go into infinite at91_twi_write_next_byte
> loop with TXRDY interrupt spam.
Sorry but I am not sure to have the full picture, the controller is in
slave or master mode?
SVREAD is only used in slave mode. When SVREAD is set, it means that a read
access is performed and your issue concerns the write operation.
Regards
Ludovic
>
> Signed-off-by: Raag Jadav
> ---
> drivers/i2c/busses/i2c-at91.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> index 3f3e8b3..b2f5fdb 100644
> --- a/drivers/i2c/busses/i2c-at91.c
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -72,6 +72,7 @@
> #define AT91_TWI_TXCOMP BIT(0) /* Transmission Complete */
> #define AT91_TWI_RXRDY BIT(1) /* Receive Holding Register
> Ready */
> #define AT91_TWI_TXRDY BIT(2) /* Transmit Holding Register
> Ready */
> +#define AT91_TWI_SVREAD BIT(3) /* Slave Read */
> #define AT91_TWI_OVRE BIT(6) /* Overrun Error */
> #define AT91_TWI_UNRE BIT(7) /* Underrun Error */
> #define AT91_TWI_NACK BIT(8) /* Not Acknowledged */
> @@ -571,7 +572,10 @@ static irqreturn_t atmel_twi_interrupt(int irq, void
> *dev_id)
> at91_disable_twi_interrupts(dev);
> complete(>cmd_complete);
> } else if (irqstatus & AT91_TWI_TXRDY) {
> - at91_twi_write_next_byte(dev);
> + if ((status & AT91_TWI_SVREAD) && (dev->buf_len == 0))
> + at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_TXRDY);
> + else
> + at91_twi_write_next_byte(dev);
> }
>
> /* catch error flags */
> --
> 2.7.4
>
>
Re: linux-next: build failure after merge of the at91 tree
On Tue, Apr 23, 2019 at 01:27:52PM +0900, Masahiro Yamada wrote:
> External E-Mail
>
>
> On Tue, Apr 23, 2019 at 10:33 AM Stephen Rothwell
> wrote:
> >
> > Hi all,
> >
> > After merging the at91 tree, today's linux-next build (arm
> > multi_v7_defconfig) failed like this:
> >
> > arch/arm/mach-at91/pm_suspend.S:17:10: fatal error: pm_data-offsets.h: No
> > such file or directory
> > #include "pm_data-offsets.h"
> > ^~~
> >
> > Caused by commit
> >
> > ab690fa1eb4b ("ARM: at91: move platform-specific asm-offset.h to
> > arch/arm/mach-at91")
> >
> > I used the version of the at91 tree from next-20190418 for today.
>
>
> Sorry, I missed to test the out-of-tree build.
>
> -I $(srctree)/$(src) is not added
> when check-in assembly files include a generated header.
> (I think this should be automatically cared by Kbuild, though.)
>
>
> Ludovic,
>
> Could you drop this patch for now?
Yes I dropped it from my PR for 5.2 and remove it from at91-next.
Regards
Ludovic
Re: [PATCH 2/3] ARM: at91: move platform-specific asm-offset.h to arch/arm/mach-at91
On Mon, Apr 15, 2019 at 05:14:50PM +0200, Alexandre Belloni wrote: > External E-Mail > > > On 08/04/2019 16:54:26+0900, Masahiro Yamada wrote: > > is only generated and included > > by arch/arm/mach-at91/, so it does not need to reside in the > > globally visible include/generated/. > > > > I moved and renamed it to arch/arm/mach-at91/pm_data-offsets.h > > since the prefix 'at91_' is just redundant in mach-at91/. > > > > Signed-off-by: Masahiro Yamada > Acked-by: Alexandre Belloni > Applied in at91-soc. Let me know if it's an issue, I plan to do the PR soon. Regards Ludovic > > --- > > > > Can this be applied to ARM-SOC tree in a series? > > (with Ack from the platform sub-maintainer.) > > > > at91_pm_data-offsets.h header does not need to reside in > > include/generated/, but you may ask > > "Why must it get out of include/generated/?" > > > > My main motivation is to avoid a race condition in the currently > > proposed patch: > > > > https://lore.kernel.org/patchwork/patch/1052763/ > > > > This patch tries to embed some build artifacts into the kernel. > > > > If arch/arm/mach-at91/ and kernel/ are built at the same time, > > it may embed a truncated file. > > > > > > arch/arm/mach-at91/.gitignore | 1 + > > arch/arm/mach-at91/Makefile | 5 +++-- > > arch/arm/mach-at91/pm_suspend.S | 2 +- > > 3 files changed, 5 insertions(+), 3 deletions(-) > > create mode 100644 arch/arm/mach-at91/.gitignore > > > > diff --git a/arch/arm/mach-at91/.gitignore b/arch/arm/mach-at91/.gitignore > > new file mode 100644 > > index ..2ecd6f51c8a9 > > --- /dev/null > > +++ b/arch/arm/mach-at91/.gitignore > > @@ -0,0 +1 @@ > > +pm_data-offsets.h > > diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile > > index 31b61f0e1c07..de64301dcff2 100644 > > --- a/arch/arm/mach-at91/Makefile > > +++ b/arch/arm/mach-at91/Makefile > > @@ -19,9 +19,10 @@ ifeq ($(CONFIG_PM_DEBUG),y) > > CFLAGS_pm.o += -DDEBUG > > endif > > > > -include/generated/at91_pm_data-offsets.h: > > arch/arm/mach-at91/pm_data-offsets.s FORCE > > +$(obj)/pm_data-offsets.h: $(obj)/pm_data-offsets.s FORCE > > $(call filechk,offsets,__PM_DATA_OFFSETS_H__) > > > > -arch/arm/mach-at91/pm_suspend.o: include/generated/at91_pm_data-offsets.h > > +$(obj)/pm_suspend.o: $(obj)/pm_data-offsets.h > > > > targets += pm_data-offsets.s > > +clean-files += pm_data-offsets.h > > diff --git a/arch/arm/mach-at91/pm_suspend.S > > b/arch/arm/mach-at91/pm_suspend.S > > index bfe1c4d06901..a31c1b20f3fa 100644 > > --- a/arch/arm/mach-at91/pm_suspend.S > > +++ b/arch/arm/mach-at91/pm_suspend.S > > @@ -14,7 +14,7 @@ > > #include > > #include > > #include "pm.h" > > -#include "generated/at91_pm_data-offsets.h" > > +#include "pm_data-offsets.h" > > > > #defineSRAMC_SELF_FRESH_ACTIVE 0x01 > > #defineSRAMC_SELF_FRESH_EXIT 0x00 > > -- > > 2.17.1 > > > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com >
Re: [PATCH v2 1/4] ARM: dts: sama5d{2,4}: use SPDX-License-Identifier
On Wed, Apr 03, 2019 at 10:52:16PM +0200, Alexandre Belloni wrote: > External E-Mail > > > The X11 license text [1] is explicitly for the X Consortium and has a > couple of extra clauses. The MIT license text [2] is actually what the > current DT files claim. > > [1] https://spdx.org/licenses/X11.html > [2] https://spdx.org/licenses/MIT.html > > Signed-off-by: Alexandre Belloni Applied to at91-dt. Thanks > --- > arch/arm/boot/dts/sama5d2.dtsi | 39 +- > arch/arm/boot/dts/sama5d4.dtsi | 39 +- > 2 files changed, 2 insertions(+), 76 deletions(-) > > diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi > index d159ee42ef29..26ae6b34e329 100644 > --- a/arch/arm/boot/dts/sama5d2.dtsi > +++ b/arch/arm/boot/dts/sama5d2.dtsi > @@ -1,46 +1,9 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > /* > * sama5d2.dtsi - Device Tree Include file for SAMA5D2 family SoC > * > * Copyright (C) 2015 Atmel, > *2015 Ludovic Desroches > - * > - * This file is dual-licensed: you can use it either under the terms > - * of the GPL or the X11 license, at your option. Note that this dual > - * licensing only applies to this file, and not this project as a > - * whole. > - * > - * a) This file is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public License as > - * published by the Free Software Foundation; either version 2 of the > - * License, or (at your option) any later version. > - * > - * This file 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. > - * > - * Or, alternatively, > - * > - * b) Permission is hereby granted, free of charge, to any person > - * obtaining a copy of this software and associated documentation > - * files (the "Software"), to deal in the Software without > - * restriction, including without limitation the rights to use, > - * copy, modify, merge, publish, distribute, sublicense, and/or > - * sell copies of the Software, and to permit persons to whom the > - * Software is furnished to do so, subject to the following > - * conditions: > - * > - * The above copyright notice and this permission notice shall be > - * included in all copies or substantial portions of the Software. > - * > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES > - * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > - * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT > - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, > - * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > - * OTHER DEALINGS IN THE SOFTWARE. > */ > > #include > diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi > index 6c1e41f94549..6ab27a7b388d 100644 > --- a/arch/arm/boot/dts/sama5d4.dtsi > +++ b/arch/arm/boot/dts/sama5d4.dtsi > @@ -1,46 +1,9 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > /* > * sama5d4.dtsi - Device Tree Include file for SAMA5D4 family SoC > * > * Copyright (C) 2014 Atmel, > *2014 Nicolas Ferre > - * > - * This file is dual-licensed: you can use it either under the terms > - * of the GPL or the X11 license, at your option. Note that this dual > - * licensing only applies to this file, and not this project as a > - * whole. > - * > - * a) This file is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public License as > - * published by the Free Software Foundation; either version 2 of the > - * License, or (at your option) any later version. > - * > - * This file 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. > - * > - * Or, alternatively, > - * > - * b) Permission is hereby granted, free of charge, to any person > - * obtaining a copy of this software and associated documentation > - * files (the "Software"), to deal in the Software without > - * restriction, including without limitation the rights to use, > - *
Re: [PATCH] ARM: at91: remove HAVE_FB_ATMEL for sama5 SoC as they use DRM
On Thu, Mar 28, 2019 at 04:33:07PM +0100, Nicolas Ferre wrote: > SAMA5 devices use the newer DRM driver for LCD. They don't need > the older FB driver: remove the Kconfig option for them. > > Signed-off-by: Nicolas Ferre Applied to at91-defconfig. Thanks. > --- > arch/arm/mach-at91/Kconfig | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig > index 903f23c309df..01b1bdb4fb6e 100644 > --- a/arch/arm/mach-at91/Kconfig > +++ b/arch/arm/mach-at91/Kconfig > @@ -21,7 +21,6 @@ config SOC_SAMA5D2 > depends on ARCH_MULTI_V7 > select SOC_SAMA5 > select CACHE_L2X0 > - select HAVE_FB_ATMEL > select HAVE_AT91_UTMI > select HAVE_AT91_USB_CLK > select HAVE_AT91_H32MX > @@ -36,7 +35,6 @@ config SOC_SAMA5D3 > bool "SAMA5D3 family" > depends on ARCH_MULTI_V7 > select SOC_SAMA5 > - select HAVE_FB_ATMEL > select HAVE_AT91_UTMI > select HAVE_AT91_SMD > select HAVE_AT91_USB_CLK > @@ -50,7 +48,6 @@ config SOC_SAMA5D4 > depends on ARCH_MULTI_V7 > select SOC_SAMA5 > select CACHE_L2X0 > - select HAVE_FB_ATMEL > select HAVE_AT91_UTMI > select HAVE_AT91_SMD > select HAVE_AT91_USB_CLK > -- > 2.17.1 >
Re: linux-next: manual merge of the at91 tree with the at91-fixes tree
Hi Stephen,
On Fri, Apr 05, 2019 at 09:15:19AM +1100, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the at91 tree got a conflict in:
>
> arch/arm/mach-at91/pm.c
>
> between commit:
>
> ba5e60c9b75d ("arm/mach-at91/pm : fix possible object reference leak")
>
> from the at91-fixes tree and commit:
>
> c3f5b8fde71f ("ARM: at91: pm: introduce at91_soc_pm structure")
>
> from the at91 tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc arch/arm/mach-at91/pm.c
> index 2a757dcaa1a5,5571658b3c46..
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@@ -620,10 -676,8 +676,10 @@@ static int __init at91_pm_backup_init(v
> return 0;
>
> securam_fail:
> +put_device(>dev);
> +securam_fail_no_ref_dev:
> - iounmap(pm_data.sfrbu);
> - pm_data.sfrbu = NULL;
> + iounmap(soc_pm.data.sfrbu);
> + soc_pm.data.sfrbu = NULL;
> return ret;
> }
>
Resolution sounds good.
Thanks
Ludovic
Re: [PATCH v4] arm/mach-at91/pm : fix possible object reference leak
On Tue, Apr 02, 2019 at 10:12:38PM +0800, Peng Hao wrote:
>
> of_find_device_by_node() takes a reference to the struct device
> when it finds a match via get_device. When returning error we should
> call put_device.
>
> Reviewed-by: Mukesh Ojha
> Signed-off-by: Peng Hao
Thanks applied to at91-fixes with:
Fixes: 24a0f5c539f9 ("ARM: at91: pm: Add sama5d2 backup mode")
Cc: # v4.14+
> ---
> arch/arm/mach-at91/pm.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 51e808a..38511a5 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -591,13 +591,13 @@ static int __init at91_pm_backup_init(void)
>
> np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-securam");
> if (!np)
> - goto securam_fail;
> + goto securam_fail_no_ref_dev;
>
> pdev = of_find_device_by_node(np);
> of_node_put(np);
> if (!pdev) {
> pr_warn("%s: failed to find securam device!\n", __func__);
> - goto securam_fail;
> + goto securam_fail_no_ref_dev;
> }
>
> sram_pool = gen_pool_get(>dev, NULL);
> @@ -620,6 +620,8 @@ static int __init at91_pm_backup_init(void)
> return 0;
>
> securam_fail:
> + put_device(>dev);
> +securam_fail_no_ref_dev:
> iounmap(pm_data.sfrbu);
> pm_data.sfrbu = NULL;
> return ret;
> --
> 1.8.3.1
>
>
Re: [PATCH] ARM: at91: remove HAVE_FB_ATMEL for sama5 SoC as they use DRM
On Fri, Mar 29, 2019 at 02:05:38PM +0100, Ludovic Desroches wrote: > On Thu, Mar 28, 2019 at 04:33:07PM +0100, Nicolas Ferre wrote: > > SAMA5 devices use the newer DRM driver for LCD. They don't need > > the older FB driver: remove the Kconfig option for them. > > > > Signed-off-by: Nicolas Ferre > Acked-by: Ludovic Desroches > > CONFIG_FB_ATMEL should be removed from at91_dt_defconfig. Do you want me > to handle it? Sorry for the noise, I confused at91_dt_defconfig and sama5_defconfig. So nothing to do. Ludovic > > > --- > > arch/arm/mach-at91/Kconfig | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig > > index 903f23c309df..01b1bdb4fb6e 100644 > > --- a/arch/arm/mach-at91/Kconfig > > +++ b/arch/arm/mach-at91/Kconfig > > @@ -21,7 +21,6 @@ config SOC_SAMA5D2 > > depends on ARCH_MULTI_V7 > > select SOC_SAMA5 > > select CACHE_L2X0 > > - select HAVE_FB_ATMEL > > select HAVE_AT91_UTMI > > select HAVE_AT91_USB_CLK > > select HAVE_AT91_H32MX > > @@ -36,7 +35,6 @@ config SOC_SAMA5D3 > > bool "SAMA5D3 family" > > depends on ARCH_MULTI_V7 > > select SOC_SAMA5 > > - select HAVE_FB_ATMEL > > select HAVE_AT91_UTMI > > select HAVE_AT91_SMD > > select HAVE_AT91_USB_CLK > > @@ -50,7 +48,6 @@ config SOC_SAMA5D4 > > depends on ARCH_MULTI_V7 > > select SOC_SAMA5 > > select CACHE_L2X0 > > - select HAVE_FB_ATMEL > > select HAVE_AT91_UTMI > > select HAVE_AT91_SMD > > select HAVE_AT91_USB_CLK > > -- > > 2.17.1 > >
Re: [PATCH] ARM: at91: remove HAVE_FB_ATMEL for sama5 SoC as they use DRM
On Thu, Mar 28, 2019 at 04:33:07PM +0100, Nicolas Ferre wrote: > SAMA5 devices use the newer DRM driver for LCD. They don't need > the older FB driver: remove the Kconfig option for them. > > Signed-off-by: Nicolas Ferre Acked-by: Ludovic Desroches CONFIG_FB_ATMEL should be removed from at91_dt_defconfig. Do you want me to handle it? > --- > arch/arm/mach-at91/Kconfig | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig > index 903f23c309df..01b1bdb4fb6e 100644 > --- a/arch/arm/mach-at91/Kconfig > +++ b/arch/arm/mach-at91/Kconfig > @@ -21,7 +21,6 @@ config SOC_SAMA5D2 > depends on ARCH_MULTI_V7 > select SOC_SAMA5 > select CACHE_L2X0 > - select HAVE_FB_ATMEL > select HAVE_AT91_UTMI > select HAVE_AT91_USB_CLK > select HAVE_AT91_H32MX > @@ -36,7 +35,6 @@ config SOC_SAMA5D3 > bool "SAMA5D3 family" > depends on ARCH_MULTI_V7 > select SOC_SAMA5 > - select HAVE_FB_ATMEL > select HAVE_AT91_UTMI > select HAVE_AT91_SMD > select HAVE_AT91_USB_CLK > @@ -50,7 +48,6 @@ config SOC_SAMA5D4 > depends on ARCH_MULTI_V7 > select SOC_SAMA5 > select CACHE_L2X0 > - select HAVE_FB_ATMEL > select HAVE_AT91_UTMI > select HAVE_AT91_SMD > select HAVE_AT91_USB_CLK > -- > 2.17.1 >
Re: [PATCH] arm/mach-at91/pm : fix possible object reference leak
Hi, On Wed, Feb 13, 2019 at 10:55:46PM +0800, Peng Hao wrote: > of_find_device_by_node() takes a reference to the struct device > when it finds a match via get_device. When returning error we should > call put_device. > > Signed-off-by: Peng Hao > --- > arch/arm/mach-at91/pm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c > index 51e808a..70fadb7 100644 > --- a/arch/arm/mach-at91/pm.c > +++ b/arch/arm/mach-at91/pm.c > @@ -621,6 +621,7 @@ static int __init at91_pm_backup_init(void) > > securam_fail: > iounmap(pm_data.sfrbu); > + put_device(>dev); Fixed in this way, in some cases (as there are goto securam_fail before calling of_find_device_by_node), we may decrement a reference which has not been incremented. Is it safe? Regards Ludovic > pm_data.sfrbu = NULL; > return ret; > } > -- > 1.8.3.1 > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH] iio: adc: at91: disable adc channel interrupt in timeout case
higher adc clock frequency (10mhz, 20 mhz) the issue doesn't show
> up (most likely because the function calls are delaying already enough). We
> also observed that the need delay time increases further according to the
> transfer time settings (TRANSFER) by 0, 2, 4 or 8 ADC cycles.
>
> This Issue can be reproduced by compiling and running the error demonstrator
> (single channel) and including a simple error message in the kernel module in
> the timeout case.
>
> $ arm-buildroot-linux-gnueabihf-gcc adc_demo_error.c -o adc_demo_error
>
> This issue is least understood by us and I think that talking to the hardware
> people regarding this issue can be worthwhile.
> As a workaround we put at91_adc_read_raw() to sleep after the writing
> AT91_ADC_CHDR for 50 to 75 ADC Cycles.
>
>
>
> We will run another round of tests next week - and after that I would like to
> prepare a pull request concerning the 2nd Issue. For the 3rd and the 4th
> Issue I would like to hear your opinion if our modifications are sound before
> I will write an pull request.
>
> Thanks for your time,
> Best wishes,
> Georg Ottinger
>
> -
>
> -Ursprüngliche Nachricht-
> Von: Jonathan Cameron [mailto:jonathan.came...@huawei.com]
> Gesendet: Montag, 04. Februar 2019 10:46
> An: Georg Ottinger
> Cc: Jonathan Cameron ; eugen.hris...@microchip.com; Stefan
> Etzlstorfer ; Hartmut Knaack ;
> Lars-Peter Clausen ; Peter Meerwald-Stadler
> ; Nicolas Ferre ; Alexandre
> Belloni ; Ludovic Desroches
> ; David S. Miller ; Ard
> Biesheuvel ; Kees Cook ;
> linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-kernel@vger.kernel.org; Maxime Ripard
> Betreff: Re: [PATCH] iio: adc: at91: disable adc channel interrupt in timeout
> case
>
> On Mon, 4 Feb 2019 07:17:07 +
> Georg Ottinger wrote:
>
> > Actually this issue occurred to us with an concrete product, where we
> > experienced a system hang at -20 °C.
> > It was triggered by a race condition between the Touch Trigger and the
> > Channel Trigger of the ADC. Once triggered we got in to the situation where
> > an ongoing Channel Conversion was lost (Timeout case).When we queried a
> > second channel than we got a system hang. Investigating this issue we
> > developed an error demonstrator - reading alternating two channels as fast
> > as possible (when Touch is enabled). This also provokes this issue at room
> > temperature.
> >
> > For the error demonstrator use following commandline to compile:
> >
> > $ arm-buildroot-linux-gnueabihf-gcc adc_demo_error.c -D2ND_CHANNEL -o
> > adc_demo_error2
> >
> > -
> > // adc_demo_error.c
> > #include
> > #include
> >
> > #define VLEN 10
> >
> > int main()
> > {
> > int fd_adc,fd_adc2;
> > int ret;
> > char dummy[VLEN];
> >
> > fd_adc = open
> > ("/sys/devices/platform/ahb/ahb:apb/f8018000.adc/iio:device0/in_voltag
> > e4_raw",O_RDONLY);
> > #ifdef 2ND_CHANNEL
> > fd_adc2 = open
> > ("/sys/devices/platform/ahb/ahb:apb/f8018000.adc/iio:device0/in_voltag
> > e5_raw",O_RDONLY);
> > #endif
> >
> > while(1) {
> >
> > lseek(fd_adc, 0, SEEK_SET);
> > ret = read(fd_adc, dummy, VLEN);
> > #ifdef 2ND_CHANNEL
> > lseek(fd_adc2, 0, SEEK_SET);
> > ret = read(fd_adc2, dummy, VLEN);
> > #endif
> >
> > }
> > }
> >
> >
> >
> >
> > Greeting, Georg
> Hi Georg,
>
> Thanks for the detailed error report and reproducer.
>
> What has me wondering now is where the race is that is triggering this?
> Could you talk us through it? I don't know this driver that well so probably
> much quicker for you to fill in the gaps rather than me try to figure out the
> race path!
>
> I have no problem with defense in depth (with appropriate comments) but I'd
> like to close that down as well. If it really is unsolvable I'd like at
> least to have some clear comments in the code explaining why.
>
> Thanks,
>
> Jonathan
>
> >
> > -Ursprüngliche Nachricht-
> > Von: Jonathan Cameron [mailto:ji...@kernel.org]
> > Gesendet: Samstag, 02. Februar 2019 11:21
> > An: Georg Ottinger
> > Cc: eugen.hris...@microchip.com; Stefan Etzlstorfer
> > ; Hartmut Knaack ;
> > Lars-Peter Clausen ; Peter Meerwald-Stadler
> > ; Nicolas Ferre ;
> > Alexandre Belloni ; Ludovic Desroches
> > ; David S. Miller
> > ; Ard Biesheuve
Re: [PATCH] iio: adc: at91: disable adc channel interrupt in timeout case
On Wed, Jan 30, 2019 at 02:42:02PM +0100, g.ottin...@abatec.at wrote:
> From: Georg Ottinger
>
> Having a brief look at at91_adc_read_raw() it is obvious that in the case
> of a timeout the setting of AT91_ADC_CHDR and AT91_ADC_IDR registers is
> omitted. If 2 different channels are queried we can end up with a
> situation where two interrupts are enabled, but only one interrupt is
> cleared in the interrupt handler. Resulting in a interrupt loop and a
> system hang.
>
> Signed-off-by: Georg Ottinger
Acked-by: Ludovic Desroches
Thanks
> ---
> drivers/iio/adc/at91_adc.c | 28 +---
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 75d2f7358..596841a3c 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -704,23 +704,29 @@ static int at91_adc_read_raw(struct iio_dev *idev,
> ret = wait_event_interruptible_timeout(st->wq_data_avail,
> st->done,
> msecs_to_jiffies(1000));
> - if (ret == 0)
> - ret = -ETIMEDOUT;
> - if (ret < 0) {
> - mutex_unlock(>lock);
> - return ret;
> - }
> -
> - *val = st->last_value;
>
> + /* Disable interrupts, regardless if adc conversion was
> + * successful or not
> + */
> at91_adc_writel(st, AT91_ADC_CHDR,
> AT91_ADC_CH(chan->channel));
> at91_adc_writel(st, AT91_ADC_IDR, BIT(chan->channel));
>
> - st->last_value = 0;
> - st->done = false;
> + if (ret > 0) {
> + /* a valid conversion took place */
> + *val = st->last_value;
> + st->last_value = 0;
> + st->done = false;
> + ret = IIO_VAL_INT;
> + } else if (ret == 0) {
> + /* conversion timeout */
> + dev_err(>dev, "ADC Channel %d timeout.\n",
> + chan->channel);
> + ret = -ETIMEDOUT;
> + }
> +
> mutex_unlock(>lock);
> - return IIO_VAL_INT;
> + return ret;
>
> case IIO_CHAN_INFO_SCALE:
> *val = st->vref_mv;
> --
> 2.17.1
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[PATCH v5 1/3] i2c: at91: segregate master mode specific code from probe and init func
From: Juergen Fitschen
In order to implement slave mode support for the at91 hardware we have to
segregate all master mode specific function parts from the general parts.
The upcoming slave mode patch will call its sepcific probe resp. init
function instead of the master mode functions after the shared general
code has been executed.
This concept has been influenced by the i2c-designware driver.
Signed-off-by: Juergen Fitschen
Signed-off-by: Ludovic Desroches
---
drivers/i2c/busses/i2c-at91.c | 90 +--
1 file changed, 55 insertions(+), 35 deletions(-)
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 3f3e8b3bf5ff..bc74184462c6 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -174,10 +174,8 @@ static void at91_twi_irq_restore(struct at91_twi_dev *dev)
at91_twi_write(dev, AT91_TWI_IER, dev->imr);
}
-static void at91_init_twi_bus(struct at91_twi_dev *dev)
+static void at91_init_twi_bus_master(struct at91_twi_dev *dev)
{
- at91_disable_twi_interrupts(dev);
- at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
/* FIFO should be enabled immediately after the software reset */
if (dev->fifo_size)
at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_FIFOEN);
@@ -186,6 +184,14 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
at91_twi_write(dev, AT91_TWI_CWGR, dev->twi_cwgr_reg);
}
+static void at91_init_twi_bus(struct at91_twi_dev *dev)
+{
+ at91_disable_twi_interrupts(dev);
+ at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
+
+ at91_init_twi_bus_master(dev);
+}
+
/*
* Calculate symmetric clock as stated in datasheet:
* twi_clk = F_MAIN / (2 * (cdiv * (1 << ckdiv) + offset))
@@ -1046,18 +1052,56 @@ static struct at91_twi_pdata *at91_twi_get_driver_data(
return (struct at91_twi_pdata *)
platform_get_device_id(pdev)->driver_data;
}
+static int at91_twi_probe_master(struct platform_device *pdev,
+u32 phy_addr, struct at91_twi_dev *dev)
+{
+ int rc;
+ u32 bus_clk_rate;
+
+ init_completion(>cmd_complete);
+
+ rc = devm_request_irq(>dev, dev->irq, atmel_twi_interrupt, 0,
+ dev_name(dev->dev), dev);
+ if (rc) {
+ dev_err(dev->dev, "Cannot get irq %d: %d\n", dev->irq, rc);
+ return rc;
+ }
+
+ if (dev->dev->of_node) {
+ rc = at91_twi_configure_dma(dev, phy_addr);
+ if (rc == -EPROBE_DEFER)
+ return rc;
+ }
+
+ if (!of_property_read_u32(pdev->dev.of_node, "atmel,fifo-size",
+ >fifo_size)) {
+ dev_info(dev->dev, "Using FIFO (%u data)\n", dev->fifo_size);
+ }
+
+ rc = of_property_read_u32(dev->dev->of_node, "clock-frequency",
+ _clk_rate);
+ if (rc)
+ bus_clk_rate = DEFAULT_TWI_CLK_HZ;
+
+ at91_calc_twi_clock(dev, bus_clk_rate);
+
+ dev->adapter.algo = _twi_algorithm;
+ dev->adapter.quirks = _twi_quirks;
+
+ return 0;
+}
+
static int at91_twi_probe(struct platform_device *pdev)
{
struct at91_twi_dev *dev;
struct resource *mem;
int rc;
u32 phy_addr;
- u32 bus_clk_rate;
dev = devm_kzalloc(>dev, sizeof(*dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
- init_completion(>cmd_complete);
+
dev->dev = >dev;
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1077,13 +1121,6 @@ static int at91_twi_probe(struct platform_device *pdev)
if (dev->irq < 0)
return dev->irq;
- rc = devm_request_irq(>dev, dev->irq, atmel_twi_interrupt, 0,
-dev_name(dev->dev), dev);
- if (rc) {
- dev_err(dev->dev, "Cannot get irq %d: %d\n", dev->irq, rc);
- return rc;
- }
-
platform_set_drvdata(pdev, dev);
dev->clk = devm_clk_get(dev->dev, NULL);
@@ -1095,38 +1132,21 @@ static int at91_twi_probe(struct platform_device *pdev)
if (rc)
return rc;
- if (dev->dev->of_node) {
- rc = at91_twi_configure_dma(dev, phy_addr);
- if (rc == -EPROBE_DEFER) {
- clk_disable_unprepare(dev->clk);
- return rc;
- }
- }
-
- if (!of_property_read_u32(pdev->dev.of_node, "atmel,fifo-size",
- >fifo_size)) {
- dev_info(dev->dev, "Using FIFO (%u data)\n", dev->fifo_size);
- }
-
- rc = of_property_read_u32(dev->dev->of_node, "clock-frequency",
-
[PATCH v5 0/3] i2c: at91: slave mode support
[Ludovic Desroches: see Changes section]
Based on the discussion we had on the i2c-linux list [1], I wrote a patch for
AT91 hardware and tried to fulfill the Linux I2C slave interface description
[2] as good as possible. This enables aforementioned hardware to act as an I2C
slave that can be accessed by a remote I2C master.
I have tested this patchset successfully on an ATSAMA5D27.
^ 3.3V ^ 3.3V
+---+| | +---+
| Slave: ATSAMA5D27 | +-+ +-+| Master: ATSAMA5D35|
| with i2c-slave-eeprom | | | 100k | | 100k | with i2cset |
+---+-+-+ +-+ +-++-+-+---+
| | | | | |
| +--+-|---(SDA)---+ |
+--+---(SCL)-+
Schematic: Connection of slave and master with 100kOhm pullup resistors.
On the master the following BASH script has been used to stress the slave.
root@emblinux:~# cat ./stress.sh
#!/bin/bash
I=0
while true
do
if i2cset -y -r 1 0x64 0 $I w | grep mismatch
then
echo "$(date): Error in transmission ${I}"
fi
((I++))
if [ $I -eq 65536 ]
then
I=0
echo "$(date): Sent 65536 transmissions"
fi
done
After running the script for some time I had the following output. To me this
looks promising :)
root@emblinux:~# ./stress.sh
Thu Nov 9 13:58:45 CTE 2017: Sent 65536 transmissions
Thu Nov 9 14:35:20 CTE 2017: Sent 65536 transmissions
Thu Nov 9 15:12:11 CTE 2017: Sent 65536 transmissions
Thu Nov 9 15:49:04 CTE 2017: Sent 65536 transmissions
Thu Nov 9 16:26:00 CTE 2017: Sent 65536 transmissions
Thu Nov 9 17:03:07 UTC 2017: Sent 65536 transmissions
Thu Nov 9 17:40:15 UTC 2017: Sent 65536 transmissions
If you have some hardware with an at91-i2c interface included at hand,
I really
would appreciate if you can run the test script on your hardware and
test this
driver.
Thu Nov 9 17:40:15 UTC 2017: Sent 65536 transmissions
If you have some hardware with an at91-i2c interface included at hand,
I really
would appreciate if you can run the test script on your hardware and
test this
driver.
Best regards
Juergen
[Ludovic Desroches]
Changes in v5:
- use SPDX for licence
- replace IF_ENABLED par defined to solve compilation issue when
selected as module.
Changes in v4:
- rebased on next-20181224
- update Kconfig part to state that this feature is experimental.
- tested quickly on a single board, SAMA5D2 Xplained, i2c-gpio as master, OK.
Changes in v3:
- rebase (cherry-pick was enough)
- fix checkpatch errors
- tests:
- hangs with a SAMA5D4 (master and slave on different busses) after about
100 transfers. It's the first time I do this test.
- some mismatches with a SAMA5D4 as slave and a SAMA5D2 as master
I don't know if it's a regression. I don't remember having seen this
behavior previously.
I think it's worth taking those patches even if there are some possible
bugs. It'll allow to get more people using it and so to consolidate the
slave mode support.
Changes in v2:
- Implemented all suggestions made by Ludovic. (Thank you!)
- Reworked the IRQ handler completely. Have a look in patch 3 fort further
details.
[1] https://marc.info/?t=15082400481=1=1
[2] https://www.kernel.org/doc/Documentation/i2c/slave-interface
Juergen Fitschen (3):
i2c: at91: segregate master mode specific code from probe and init
func
i2c: at91: split driver into core and master file
i2c: at91: added slave mode support
MAINTAINERS | 3 +-
drivers/i2c/busses/Kconfig| 13 +
drivers/i2c/busses/Makefile | 4 +
drivers/i2c/busses/i2c-at91-core.c| 376 ++
.../busses/{i2c-at91.c => i2c-at91-master.c} | 459 +-
drivers/i2c/busses/i2c-at91-slave.c | 143 ++
drivers/i2c/busses/i2c-at91.h | 175 +++
7 files changed, 721 insertions(+), 452 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-at91-core.c
rename drivers/i2c/busses/{i2c-at91.c => i2c-at91-master.c} (66%)
create mode 100644 drivers/i2c/busses/i2c-at91-slave.c
create mode 100644 drivers/i2c/busses/i2c-at91.h
--
2.20.1
[PATCH v5 2/3] i2c: at91: split driver into core and master file
From: Juergen Fitschen
The single file i2c-at91.c has been split into core code (i2c-at91-core.c)
and master mode specific code (i2c-at91-master.c). This should enhance
maintainability and reduce ifdeffery for slave mode related code.
The code itself hasn't been touched. Shared functions only had to be made
non-static. Furthermore, includes have been cleaned up.
Signed-off-by: Juergen Fitschen
[ludovic.desroc...@microchip.com: fix checkpatch errors and use SPDX]
Signed-off-by: Ludovic Desroches
---
MAINTAINERS | 3 +-
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-at91-core.c| 369 ++
.../busses/{i2c-at91.c => i2c-at91-master.c} | 473 +-
drivers/i2c/busses/i2c-at91.h | 147 ++
5 files changed, 524 insertions(+), 469 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-at91-core.c
rename drivers/i2c/busses/{i2c-at91.c => i2c-at91-master.c} (66%)
create mode 100644 drivers/i2c/busses/i2c-at91.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 32d76a90..1dafe6ef4f6d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9890,7 +9890,8 @@ MICROCHIP I2C DRIVER
M: Ludovic Desroches
L: linux-...@vger.kernel.org
S: Supported
-F: drivers/i2c/busses/i2c-at91.c
+F: drivers/i2c/busses/i2c-at91.h
+F: drivers/i2c/busses/i2c-at91-*.c
MICROCHIP ISC DRIVER
M: Eugen Hristev
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 5f0cb6915969..ea75a777940e 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_I2C_POWERMAC)+= i2c-powermac.o
obj-$(CONFIG_I2C_ALTERA) += i2c-altera.o
obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
+i2c-at91-objs := i2c-at91-core.o i2c-at91-master.o
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_AXXIA)+= i2c-axxia.o
obj-$(CONFIG_I2C_BCM2835) += i2c-bcm2835.o
diff --git a/drivers/i2c/busses/i2c-at91-core.c
b/drivers/i2c/busses/i2c-at91-core.c
new file mode 100644
index ..6cb22341fa81
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
+ *
+ * Copyright (C) 2011 Weinmann Medical GmbH
+ * Author: Nikolaus Voss
+ *
+ * Evolved from original work by:
+ * Copyright (C) 2004 Rick Bronson
+ * Converted to 2.6 by Andrew Victor
+ *
+ * Borrowed heavily from original work by:
+ * Copyright (C) 2000 Philip Edelbrock
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "i2c-at91.h"
+
+unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg)
+{
+ return readl_relaxed(dev->base + reg);
+}
+
+void at91_twi_write(struct at91_twi_dev *dev, unsigned reg, unsigned val)
+{
+ writel_relaxed(val, dev->base + reg);
+}
+
+void at91_disable_twi_interrupts(struct at91_twi_dev *dev)
+{
+ at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_INT_MASK);
+}
+
+void at91_twi_irq_save(struct at91_twi_dev *dev)
+{
+ dev->imr = at91_twi_read(dev, AT91_TWI_IMR) & AT91_TWI_INT_MASK;
+ at91_disable_twi_interrupts(dev);
+}
+
+void at91_twi_irq_restore(struct at91_twi_dev *dev)
+{
+ at91_twi_write(dev, AT91_TWI_IER, dev->imr);
+}
+
+void at91_init_twi_bus(struct at91_twi_dev *dev)
+{
+ at91_disable_twi_interrupts(dev);
+ at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
+
+ at91_init_twi_bus_master(dev);
+}
+
+static struct at91_twi_pdata at91rm9200_config = {
+ .clk_max_div = 5,
+ .clk_offset = 3,
+ .has_unre_flag = true,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9261_config = {
+ .clk_max_div = 5,
+ .clk_offset = 4,
+ .has_unre_flag = false,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9260_config = {
+ .clk_max_div = 7,
+ .clk_offset = 4,
+ .has_unre_flag = false,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9g20_config = {
+ .clk_max_div = 7,
+ .clk_offset = 4,
+ .has_unre_flag = false,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9g10_config = {
+ .clk_max_div = 7,
+ .clk_offset = 4,
+ .has_unre_flag = false,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static const struct platform_device_id at91_twi_devtypes[] = {
+ {
+ .name = "i2c-at91rm9200",
+ .driver_data = (unsigned long) _config,
+ }, {
+ .name = "i2c-at91sam9261",
+
[PATCH v5 3/3] i2c: at91: added slave mode support
From: Juergen Fitschen
Slave mode driver is based on the concept of i2c-designware driver.
Signed-off-by: Juergen Fitschen
[ludovic.desroc...@microchip.com: rework Kconfig and replace IS_ENABLED
by defined]
Signed-off-by: Ludovic Desroches
---
drivers/i2c/busses/Kconfig | 13 +++
drivers/i2c/busses/Makefile | 3 +
drivers/i2c/busses/i2c-at91-core.c | 13 ++-
drivers/i2c/busses/i2c-at91-slave.c | 143
drivers/i2c/busses/i2c-at91.h | 30 +-
5 files changed, 198 insertions(+), 4 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-at91-slave.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index f2c681971201..6b1f6dcdf533 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -387,6 +387,19 @@ config I2C_AT91
the latency to fill the transmission register is too long. If you
are facing this situation, use the i2c-gpio driver.
+config I2C_AT91_SLAVE_EXPERIMENTAL
+ tristate "Microchip AT91 I2C experimental slave mode"
+ depends on I2C_AT91
+ select I2C_SLAVE
+ help
+ If you say yes to this option, support for the slave mode will be
+ added. Caution: do not use it for production. This feature has not
+ been tested in a heavy way, help wanted.
+ There are known bugs:
+ - It can hang, on a SAMA5D4, after several transfers.
+ - There are some mismtaches with a SAMA5D4 as slave and a SAMA5D2 as
+ master.
+
config I2C_AU1550
tristate "Au1550/Au1200/Au1300 SMBus interface"
depends on MIPS_ALCHEMY
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index ea75a777940e..59b22fbef90a 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -36,6 +36,9 @@ obj-$(CONFIG_I2C_ALTERA) += i2c-altera.o
obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
i2c-at91-objs := i2c-at91-core.o i2c-at91-master.o
+ifeq ($(CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL),y)
+ i2c-at91-objs += i2c-at91-slave.o
+endif
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_AXXIA)+= i2c-axxia.o
obj-$(CONFIG_I2C_BCM2835) += i2c-bcm2835.o
diff --git a/drivers/i2c/busses/i2c-at91-core.c
b/drivers/i2c/busses/i2c-at91-core.c
index 6cb22341fa81..8d55cdd69ff4 100644
--- a/drivers/i2c/busses/i2c-at91-core.c
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -56,8 +56,10 @@ void at91_init_twi_bus(struct at91_twi_dev *dev)
{
at91_disable_twi_interrupts(dev);
at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
-
- at91_init_twi_bus_master(dev);
+ if (dev->slave_detected)
+ at91_init_twi_bus_slave(dev);
+ else
+ at91_init_twi_bus_master(dev);
}
static struct at91_twi_pdata at91rm9200_config = {
@@ -239,7 +241,12 @@ static int at91_twi_probe(struct platform_device *pdev)
dev->adapter.timeout = AT91_I2C_TIMEOUT;
dev->adapter.dev.of_node = pdev->dev.of_node;
- rc = at91_twi_probe_master(pdev, phy_addr, dev);
+ dev->slave_detected = i2c_detect_slave_mode(>dev);
+
+ if (dev->slave_detected)
+ rc = at91_twi_probe_slave(pdev, phy_addr, dev);
+ else
+ rc = at91_twi_probe_master(pdev, phy_addr, dev);
if (rc)
return rc;
diff --git a/drivers/i2c/busses/i2c-at91-slave.c
b/drivers/i2c/busses/i2c-at91-slave.c
new file mode 100644
index ..d6eeea5166c0
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91-slave.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * i2c slave support for Atmel's AT91 Two-Wire Interface (TWI)
+ *
+ * Copyright (C) 2017 Juergen Fitschen
+ */
+
+#include
+#include
+#include
+#include
+
+#include "i2c-at91.h"
+
+static irqreturn_t atmel_twi_interrupt_slave(int irq, void *dev_id)
+{
+ struct at91_twi_dev *dev = dev_id;
+ const unsigned status = at91_twi_read(dev, AT91_TWI_SR);
+ const unsigned irqstatus = status & at91_twi_read(dev, AT91_TWI_IMR);
+ u8 value;
+
+ if (!irqstatus)
+ return IRQ_NONE;
+
+ /* slave address has been detected on I2C bus */
+ if (irqstatus & AT91_TWI_SVACC) {
+ if (status & AT91_TWI_SVREAD) {
+ i2c_slave_event(dev->slave,
+ I2C_SLAVE_READ_REQUESTED, );
+ writeb_relaxed(value, dev->base + AT91_TWI_THR);
+ at91_twi_write(dev, AT91_TWI_IER,
+ AT91_TWI_TXRDY | AT91_TWI_EOSACC);
+ } else {
+ i2c_slave_event(dev->slave,
+ I2C_SLAVE_WRITE_REQUESTED, );
+ at91_twi_write(dev, AT91_TWI_IER,
Re: [PATCH v4 3/3] i2c: at91: added slave mode support
Hi, On Fri, Feb 15, 2019 at 10:18:47AM +0100, Wolfram Sang wrote: > On Tue, Jan 15, 2019 at 11:43:51PM +0100, Wolfram Sang wrote: > > > > > All errors (new ones prefixed by >>): > > > > > > >> ERROR: "at91_init_twi_bus_slave" [drivers/i2c/busses/i2c-at91.ko] > > > >> undefined! > > > >> ERROR: "at91_twi_probe_slave" [drivers/i2c/busses/i2c-at91.ko] > > > >> undefined! > > > > That needs to be fixed. Rest looks good to me! > > Ludovic, do you have time to fix it? I would really like to apply this > series for the next merge window. > Sorry it's in my todo list but I didn't have time to handle it. I'll try to fix it beginning of next week. Regards Ludovic > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 1/7] pinctrl: at91: add option to use drive strength bits
On Wed, Feb 06, 2019 at 09:46:28AM +, claudiu.bez...@microchip.com wrote:
>
>
> On 06.02.2019 10:13, Ludovic Desroches wrote:
> > On Thu, Jan 31, 2019 at 05:18:04PM +0100, Claudiu Beznea - M18063 wrote:
> >> From: Claudiu Beznea
> >>
> >> SAM9X60 uses high and low drive strengths. To implement this, in
> >> at91_pinctrl_mux_ops::set_drivestrength and
> >> at91_pinctrl_mux_ops::get_drivestrength we need bit numbers of
> >> drive strengths (1 for low, 2 for high), thus change the code to
> >> allow the usage of drive strength bit numbers.
> >>
> >> Signed-off-by: Claudiu Beznea
> >> ---
> >> drivers/pinctrl/pinctrl-at91.c | 32
> >> 1 file changed, 20 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/pinctrl-at91.c
> >> b/drivers/pinctrl/pinctrl-at91.c
> >> index 3d49bbbcdbc7..31f06dafca2e 100644
> >> --- a/drivers/pinctrl/pinctrl-at91.c
> >> +++ b/drivers/pinctrl/pinctrl-at91.c
> >> @@ -72,10 +72,15 @@ static int gpio_banks;
> >> * DRIVE_STRENGTH_DEFAULT is just a placeholder to avoid changing the
> >> drive
> >> * strength when there is no dt config for it.
> >> */
> >> -#define DRIVE_STRENGTH_DEFAULT(0 << DRIVE_STRENGTH_SHIFT)
> >> -#define DRIVE_STRENGTH_LOW (1 << DRIVE_STRENGTH_SHIFT)
> >> -#define DRIVE_STRENGTH_MED (2 << DRIVE_STRENGTH_SHIFT)
> >> -#define DRIVE_STRENGTH_HI (3 << DRIVE_STRENGTH_SHIFT)
> >> +enum drive_strength_bit {
> >> + DRIVE_STRENGTH_BIT_DEF,
> >> + DRIVE_STRENGTH_BIT_LOW,
> >> + DRIVE_STRENGTH_BIT_MED,
> >> + DRIVE_STRENGTH_BIT_HI,
> >> +};
> >> +
> >> +#define DRIVE_STRENGTH_BIT_MSK(name) (DRIVE_STRENGTH_BIT_##name << \
> >> + DRIVE_STRENGTH_SHIFT)
> >>
> >> /**
> >> * struct at91_pmx_func - describes AT91 pinmux functions
> >> @@ -551,7 +556,7 @@ static unsigned
> >> at91_mux_sama5d3_get_drivestrength(void __iomem *pio,
> >>/* SAMA5 strength is 1:1 with our defines,
> >> * except 0 is equivalent to low per datasheet */
> >>if (!tmp)
> >> - tmp = DRIVE_STRENGTH_LOW;
> >> + tmp = DRIVE_STRENGTH_BIT_MSK(LOW);
> >>
> >>return tmp;
> >> }
> >> @@ -564,7 +569,7 @@ static unsigned at91_mux_sam9x5_get_drivestrength(void
> >> __iomem *pio,
> >>
> >>/* strength is inverse in SAM9x5s hardware with the pinctrl defines
> >> * hardware: 0 = hi, 1 = med, 2 = low, 3 = rsvd */
> >> - tmp = DRIVE_STRENGTH_HI - tmp;
> >> + tmp = DRIVE_STRENGTH_BIT_MSK(HI) - tmp;
> >>
> >>return tmp;
> >> }
> >> @@ -600,7 +605,7 @@ static void at91_mux_sam9x5_set_drivestrength(void
> >> __iomem *pio, unsigned pin,
> >>
> >>/* strength is inverse on SAM9x5s with our defines
> >> * 0 = hi, 1 = med, 2 = low, 3 = rsvd */
> >> - setting = DRIVE_STRENGTH_HI - setting;
> >> + setting = DRIVE_STRENGTH_BIT_MSK(HI) - setting;
> >>
> >>set_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin,
> >>setting);
> >> @@ -959,11 +964,11 @@ static int at91_pinconf_set(struct pinctrl_dev
> >> *pctldev,
> >>} \
> >> } while (0)
> >>
> >> -#define DBG_SHOW_FLAG_MASKED(mask,flag) do { \
> >> +#define DBG_SHOW_FLAG_MASKED(mask,flag,name) do { \
> >
> > checkpatch error: space required
>
> I'm aware of that but I added it in the way it was previously (were the
> checkpatch error was still present). Please let me know if you want me to
> send a new version fixing this.
I think it's the opportunity to fix it. If you can resend a new version
including my ack.
Regards
Ludovic
>
> Thank you,
> Claudiu Beznea
>
> >
> >>if ((config & mask) == flag) { \
> >>if (num_conf) \
> >>seq_puts(s, "|"); \
> >> - seq_puts(s, #flag); \
> >> + seq_puts(s, #name); \
> >>num_conf++; \
> >>} \
> >> } while (0)
> >> @@ -981,9 +986,12 @@ static vo
Re: [PATCH v2 0/7] add support for SAM9X60 pin controller
On Thu, Jan 31, 2019 at 05:18:00PM +0100, Claudiu Beznea - M18063 wrote: > From: Claudiu Beznea Minor comment for patch 1. Acked-by: Ludovic Desroches for the whole series. > > This series adds drive strenght and slew rate support for SAMX60's pin > controller. For drive strenght we could have 2 values: low, high. > For slew rate we could have 2 values: enable, disabled. > > Besides this I took the chance and adapt the documentation for at91 pinctrl > driver. > > Changes in v2: > - s/microchip,sam9x60-pictrl/microchip,sam9x60-pinctrl in patches 3/7, 5/7 > > Claudiu Beznea (7): > pinctrl: at91: add option to use drive strength bits > pinctrl: at91: add drive strength support for SAM9X60 > pinctrl: at91: add compatibles for SAM9X60 pin controller > dt-bindings: add documentation for banks > dt-bindings: add bindings for SAM9X60 > pinctrl: at91: add slewrate support for SAM9X60 > dt-bindings: add documentation for slew rate > > .../bindings/pinctrl/atmel,at91-pinctrl.txt| 27 - > drivers/pinctrl/pinctrl-at91.c | 134 > +++-- > drivers/pinctrl/pinctrl-at91.h | 3 + > include/dt-bindings/pinctrl/at91.h | 4 + > 4 files changed, 155 insertions(+), 13 deletions(-) > > -- > 2.7.4 >
Re: [PATCH v2 1/7] pinctrl: at91: add option to use drive strength bits
On Thu, Jan 31, 2019 at 05:18:04PM +0100, Claudiu Beznea - M18063 wrote:
> From: Claudiu Beznea
>
> SAM9X60 uses high and low drive strengths. To implement this, in
> at91_pinctrl_mux_ops::set_drivestrength and
> at91_pinctrl_mux_ops::get_drivestrength we need bit numbers of
> drive strengths (1 for low, 2 for high), thus change the code to
> allow the usage of drive strength bit numbers.
>
> Signed-off-by: Claudiu Beznea
> ---
> drivers/pinctrl/pinctrl-at91.c | 32
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 3d49bbbcdbc7..31f06dafca2e 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -72,10 +72,15 @@ static int gpio_banks;
> * DRIVE_STRENGTH_DEFAULT is just a placeholder to avoid changing the drive
> * strength when there is no dt config for it.
> */
> -#define DRIVE_STRENGTH_DEFAULT (0 << DRIVE_STRENGTH_SHIFT)
> -#define DRIVE_STRENGTH_LOW (1 << DRIVE_STRENGTH_SHIFT)
> -#define DRIVE_STRENGTH_MED (2 << DRIVE_STRENGTH_SHIFT)
> -#define DRIVE_STRENGTH_HI (3 << DRIVE_STRENGTH_SHIFT)
> +enum drive_strength_bit {
> + DRIVE_STRENGTH_BIT_DEF,
> + DRIVE_STRENGTH_BIT_LOW,
> + DRIVE_STRENGTH_BIT_MED,
> + DRIVE_STRENGTH_BIT_HI,
> +};
> +
> +#define DRIVE_STRENGTH_BIT_MSK(name) (DRIVE_STRENGTH_BIT_##name << \
> + DRIVE_STRENGTH_SHIFT)
>
> /**
> * struct at91_pmx_func - describes AT91 pinmux functions
> @@ -551,7 +556,7 @@ static unsigned at91_mux_sama5d3_get_drivestrength(void
> __iomem *pio,
> /* SAMA5 strength is 1:1 with our defines,
>* except 0 is equivalent to low per datasheet */
> if (!tmp)
> - tmp = DRIVE_STRENGTH_LOW;
> + tmp = DRIVE_STRENGTH_BIT_MSK(LOW);
>
> return tmp;
> }
> @@ -564,7 +569,7 @@ static unsigned at91_mux_sam9x5_get_drivestrength(void
> __iomem *pio,
>
> /* strength is inverse in SAM9x5s hardware with the pinctrl defines
>* hardware: 0 = hi, 1 = med, 2 = low, 3 = rsvd */
> - tmp = DRIVE_STRENGTH_HI - tmp;
> + tmp = DRIVE_STRENGTH_BIT_MSK(HI) - tmp;
>
> return tmp;
> }
> @@ -600,7 +605,7 @@ static void at91_mux_sam9x5_set_drivestrength(void
> __iomem *pio, unsigned pin,
>
> /* strength is inverse on SAM9x5s with our defines
>* 0 = hi, 1 = med, 2 = low, 3 = rsvd */
> - setting = DRIVE_STRENGTH_HI - setting;
> + setting = DRIVE_STRENGTH_BIT_MSK(HI) - setting;
>
> set_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin,
> setting);
> @@ -959,11 +964,11 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> } \
> } while (0)
>
> -#define DBG_SHOW_FLAG_MASKED(mask,flag) do { \
> +#define DBG_SHOW_FLAG_MASKED(mask,flag,name) do {\
checkpatch error: space required
> if ((config & mask) == flag) { \
> if (num_conf) \
> seq_puts(s, "|"); \
> - seq_puts(s, #flag); \
> + seq_puts(s, #name); \
> num_conf++; \
> } \
> } while (0)
> @@ -981,9 +986,12 @@ static void at91_pinconf_dbg_show(struct pinctrl_dev
> *pctldev,
> DBG_SHOW_FLAG(PULL_DOWN);
> DBG_SHOW_FLAG(DIS_SCHMIT);
> DBG_SHOW_FLAG(DEGLITCH);
> - DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_LOW);
> - DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_MED);
> - DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_HI);
> + DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(LOW),
> + DRIVE_STRENGTH_LOW);
> + DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(MED),
> + DRIVE_STRENGTH_MED);
> + DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(HI),
> + DRIVE_STRENGTH_HI);
> DBG_SHOW_FLAG(DEBOUNCE);
> if (config & DEBOUNCE) {
> val = config >> DEBOUNCE_VAL_SHIFT;
> --
> 2.7.4
>
Re: [PATCH 3/3] dmaengine: at_xdmac: only monitor overflow errors for peripheral xfer
On Tue, Feb 05, 2019 at 12:03:43PM +0100, Nicolas Ferre wrote:
> The overflow error flag (ROI: Request Overflow Error) is only relevant
> for the case when the channel handles a peripheral synchronized transfer.
> Not in the case of memory to memory transfer where there is no hardware
> request signal.
>
> Remove the use of this interrupt source in such a case. It's based on
> the first descriptor which holds the configuration for the whole
> linked list transfer.
>
> Signed-off-by: Nicolas Ferre
Acked-by: Ludovic Desroches
> ---
> drivers/dma/at_xdmac.c | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index ec7a29d8e448..b558a23ffbc2 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -308,6 +308,11 @@ static inline int at_xdmac_csize(u32 maxburst)
> return csize;
> };
>
> +static inline bool at_xdmac_chan_is_peripheral_xfer(u32 cfg)
> +{
> + return cfg & AT_XDMAC_CC_TYPE_PER_TRAN;
> +}
> +
> static inline u8 at_xdmac_get_dwidth(u32 cfg)
> {
> return (cfg & AT_XDMAC_CC_DWIDTH_MASK) >> AT_XDMAC_CC_DWIDTH_OFFSET;
> @@ -389,7 +394,13 @@ static void at_xdmac_start_xfer(struct at_xdmac_chan
> *atchan,
>at_xdmac_chan_read(atchan, AT_XDMAC_CUBC));
>
> at_xdmac_chan_write(atchan, AT_XDMAC_CID, 0x);
> - reg = AT_XDMAC_CIE_RBEIE | AT_XDMAC_CIE_WBEIE | AT_XDMAC_CIE_ROIE;
> + reg = AT_XDMAC_CIE_RBEIE | AT_XDMAC_CIE_WBEIE;
> + /*
> + * Request Overflow Error is only for peripheral synchronized transfers
> + */
> + if (at_xdmac_chan_is_peripheral_xfer(first->lld.mbr_cfg))
> + reg |= AT_XDMAC_CIE_ROIE;
> +
> /*
>* There is no end of list when doing cyclic dma, we need to get
>* an interrupt after each periods.
> --
> 2.17.1
>
Re: [PATCH 2/3] dmaengine: at_xdmac: enhance channel errors handling in tasklet
On Tue, Feb 05, 2019 at 12:03:42PM +0100, Nicolas Ferre wrote:
> Complement the identification of errors with stoping the channel and
> dumping the descriptor that led to the error case.
>
> Signed-off-by: Nicolas Ferre
Acked-by: Ludovic Desroches
> ---
> drivers/dma/at_xdmac.c | 43 --
> 1 file changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index 37a269420435..ec7a29d8e448 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -1575,6 +1575,41 @@ static void at_xdmac_handle_cyclic(struct
> at_xdmac_chan *atchan)
> dmaengine_desc_get_callback_invoke(txd, NULL);
> }
>
> +static void at_xdmac_handle_error(struct at_xdmac_chan *atchan)
> +{
> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
> + struct at_xdmac_desc*bad_desc;
> +
> + /*
> + * The descriptor currently at the head of the active list is
> + * broked. Since we don't have any way to report errors, we'll
> + * just have to scream loudly and try to carry on.
> + */
> + if (atchan->irq_status & AT_XDMAC_CIS_RBEIS)
> + dev_err(chan2dev(>chan), "read bus error!!!");
> + if (atchan->irq_status & AT_XDMAC_CIS_WBEIS)
> + dev_err(chan2dev(>chan), "write bus error!!!");
> + if (atchan->irq_status & AT_XDMAC_CIS_ROIS)
> + dev_err(chan2dev(>chan), "request overflow error!!!");
> +
> + spin_lock_bh(>lock);
> + /* Channel must be disabled first as it's not done automatically */
> + at_xdmac_write(atxdmac, AT_XDMAC_GD, atchan->mask);
> + while (at_xdmac_read(atxdmac, AT_XDMAC_GS) & atchan->mask)
> + cpu_relax();
> + bad_desc = list_first_entry(>xfers_list,
> + struct at_xdmac_desc,
> + xfer_node);
> + spin_unlock_bh(>lock);
> + /* Print bad descriptor's details if needed */
> + dev_dbg(chan2dev(>chan),
> + "%s: lld: mbr_sa=%pad, mbr_da=%pad, mbr_ubc=0x%08x\n",
> + __func__, _desc->lld.mbr_sa, _desc->lld.mbr_da,
> + bad_desc->lld.mbr_ubc);
> +
> + /* Then continue with usual descriptor management */
> +}
> +
> static void at_xdmac_tasklet(unsigned long data)
> {
> struct at_xdmac_chan*atchan = (struct at_xdmac_chan *)data;
> @@ -1594,12 +1629,8 @@ static void at_xdmac_tasklet(unsigned long data)
> || (atchan->irq_status & error_mask)) {
> struct dma_async_tx_descriptor *txd;
>
> - if (atchan->irq_status & AT_XDMAC_CIS_RBEIS)
> - dev_err(chan2dev(>chan), "read bus error!!!");
> - if (atchan->irq_status & AT_XDMAC_CIS_WBEIS)
> - dev_err(chan2dev(>chan), "write bus error!!!");
> - if (atchan->irq_status & AT_XDMAC_CIS_ROIS)
> - dev_err(chan2dev(>chan), "request overflow
> error!!!");
> + if (atchan->irq_status & error_mask)
> + at_xdmac_handle_error(atchan);
>
> spin_lock(>lock);
> desc = list_first_entry(>xfers_list,
> --
> 2.17.1
>
Re: [PATCH 1/3] dmaengine: at_xdmac: remove BUG_ON macro in tasklet
On Tue, Feb 05, 2019 at 12:03:41PM +0100, Nicolas Ferre wrote:
> Even if this case shouldn't happen when controller is properly programmed,
> it's still better to avoid dumping a kernel Oops for this.
> As the sequence may happen only for debugging purposes, log the error and
> just finish the tasklet call.
>
> Signed-off-by: Nicolas Ferre
Acked-by: Ludovic Desroches
Thanks
> ---
> drivers/dma/at_xdmac.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index fe69dccfa0c0..37a269420435 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -1606,7 +1606,11 @@ static void at_xdmac_tasklet(unsigned long data)
> struct at_xdmac_desc,
> xfer_node);
> dev_vdbg(chan2dev(>chan), "%s: desc 0x%p\n", __func__,
> desc);
> - BUG_ON(!desc->active_xfer);
> + if (!desc->active_xfer) {
> + dev_err(chan2dev(>chan), "Xfer not active:
> exiting");
> + spin_unlock_bh(>lock);
> + return;
> + }
>
> txd = >tx_dma_desc;
>
> --
> 2.17.1
>
Re: [PATCH 5/7] dt-bindings: add bindings for SAM9X60
On Thu, Jan 31, 2019 at 01:29:40PM +0100, Claudiu Beznea - M18063 wrote: > From: Claudiu Beznea > > Add device tree binding for SAM9X60 pin controller. > > Signed-off-by: Claudiu Beznea > --- > Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt > b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt > index 40e33dfc36fd..c2d51ed86d47 100644 > --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt > +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt > @@ -19,7 +19,7 @@ such as pull-up, multi drive, etc. > > Required properties for iomux controller: > - compatible: "atmel,at91rm9200-pinctrl" or "atmel,at91sam9x5-pinctrl" > - or "atmel,sama5d3-pinctrl" > + or "atmel,sama5d3-pinctrl" or "microchip,sam9x60-pictrl" Typo s/microchip,sam9x60-pictrl/microchip,sam9x60-pinctrl > - atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be >configured in this periph mode. All the periph and bank need to be > describe. > > @@ -117,7 +117,8 @@ Some requirements for using atmel,at91rm9200-pinctrl > binding: > 4. The gpio controller must be describe in the pinctrl simple-bus. > > For each bank the required properties are: > -- compatible: "atmel,at91sam9x5-gpio" or "atmel,at91rm9200-gpio" > +- compatible: "atmel,at91sam9x5-gpio" or "atmel,at91rm9200-gpio" or > + "microchip,sam9x60-gpio" > - reg: physical base address and length of the controller's registers > - interrupts: interrupt outputs from the controller > - interrupt-controller: marks the device node as an interrupt controller > -- > 2.7.4 >
Re: [PATCH 3/7] pinctrl: at91: add compatibles for SAM9X60 pin controller
On Thu, Jan 31, 2019 at 01:29:33PM +0100, Claudiu Beznea - M18063 wrote:
> From: Claudiu Beznea
>
> Add compatibles for SAM9X60 pin controller.
>
> Signed-off-by: Claudiu Beznea
> ---
> drivers/pinctrl/pinctrl-at91.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 46443b97d811..5456a2692b8c 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -1215,6 +1215,7 @@ static const struct of_device_id
> at91_pinctrl_of_match[] = {
> { .compatible = "atmel,sama5d3-pinctrl", .data = _ops },
> { .compatible = "atmel,at91sam9x5-pinctrl", .data = _ops },
> { .compatible = "atmel,at91rm9200-pinctrl", .data = _ops },
> + { .compatible = "microchip,sam9x60-pictrl", .data = _ops },
Typo s/microchip,sam9x60-pictrl/microchip,sam9x60-pinctrl
> { /* sentinel */ }
> };
>
> @@ -1757,6 +1758,7 @@ static const struct gpio_chip at91_gpio_template = {
> static const struct of_device_id at91_gpio_of_match[] = {
> { .compatible = "atmel,at91sam9x5-gpio", .data = _ops, },
> { .compatible = "atmel,at91rm9200-gpio", .data = _ops },
> + { .compatible = "microchip,sam9x60-gpio", .data = _ops },
> { /* sentinel */ }
> };
>
> --
> 2.7.4
>
Re: [PATCH] dmaengine: at_xdmac: Fix wrongfull report of a channel as in use
On Thu, Jan 17, 2019 at 05:10:38PM +0100, Codrin Ciubotariu - M19940 wrote:
> From: Codrin Ciubotariu
>
> atchan->status is used for two things:
> - pass channel interrupts status from interrupt handler to tasklet;
> - channel information like whether it is cyclic or paused;
>
> Since these operations have nothing in common, this patch adds a
> different struct member to keep the interrupts status.
>
> Fixes a bug in which a channel is wrongfully reported as in use when
> trying to obtain a new descriptior for a previously used cyclic channel.
>
> Signed-off-by: Codrin Ciubotariu
Acked-by: Ludovic Desroches
As Vinod suggested, you may change the commit message to emphasize
that the bug comes from the fact that a single variable is used to store
different information.
The Fixes: tag should be added too.
Regards
Ludovic
> ---
> drivers/dma/at_xdmac.c | 19 ++-
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index 4e557684f792..fe69dccfa0c0 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -203,6 +203,7 @@ struct at_xdmac_chan {
> u32 save_cim;
> u32 save_cnda;
> u32 save_cndc;
> + u32 irq_status;
> unsigned long status;
> struct tasklet_struct tasklet;
> struct dma_slave_config sconfig;
> @@ -1580,8 +1581,8 @@ static void at_xdmac_tasklet(unsigned long data)
> struct at_xdmac_desc*desc;
> u32 error_mask;
>
> - dev_dbg(chan2dev(>chan), "%s: status=0x%08lx\n",
> - __func__, atchan->status);
> + dev_dbg(chan2dev(>chan), "%s: status=0x%08x\n",
> + __func__, atchan->irq_status);
>
> error_mask = AT_XDMAC_CIS_RBEIS
>| AT_XDMAC_CIS_WBEIS
> @@ -1589,15 +1590,15 @@ static void at_xdmac_tasklet(unsigned long data)
>
> if (at_xdmac_chan_is_cyclic(atchan)) {
> at_xdmac_handle_cyclic(atchan);
> - } else if ((atchan->status & AT_XDMAC_CIS_LIS)
> -|| (atchan->status & error_mask)) {
> + } else if ((atchan->irq_status & AT_XDMAC_CIS_LIS)
> +|| (atchan->irq_status & error_mask)) {
> struct dma_async_tx_descriptor *txd;
>
> - if (atchan->status & AT_XDMAC_CIS_RBEIS)
> + if (atchan->irq_status & AT_XDMAC_CIS_RBEIS)
> dev_err(chan2dev(>chan), "read bus error!!!");
> - if (atchan->status & AT_XDMAC_CIS_WBEIS)
> + if (atchan->irq_status & AT_XDMAC_CIS_WBEIS)
> dev_err(chan2dev(>chan), "write bus error!!!");
> - if (atchan->status & AT_XDMAC_CIS_ROIS)
> + if (atchan->irq_status & AT_XDMAC_CIS_ROIS)
> dev_err(chan2dev(>chan), "request overflow
> error!!!");
>
> spin_lock(>lock);
> @@ -1652,7 +1653,7 @@ static irqreturn_t at_xdmac_interrupt(int irq, void
> *dev_id)
> atchan = >chan[i];
> chan_imr = at_xdmac_chan_read(atchan, AT_XDMAC_CIM);
> chan_status = at_xdmac_chan_read(atchan, AT_XDMAC_CIS);
> - atchan->status = chan_status & chan_imr;
> + atchan->irq_status = chan_status & chan_imr;
> dev_vdbg(atxdmac->dma.dev,
>"%s: chan%d: imr=0x%x, status=0x%x\n",
>__func__, i, chan_imr, chan_status);
> @@ -1666,7 +1667,7 @@ static irqreturn_t at_xdmac_interrupt(int irq, void
> *dev_id)
>at_xdmac_chan_read(atchan, AT_XDMAC_CDA),
>at_xdmac_chan_read(atchan, AT_XDMAC_CUBC));
>
> - if (atchan->status & (AT_XDMAC_CIS_RBEIS |
> AT_XDMAC_CIS_WBEIS))
> + if (atchan->irq_status & (AT_XDMAC_CIS_RBEIS |
> AT_XDMAC_CIS_WBEIS))
> at_xdmac_write(atxdmac, AT_XDMAC_GD,
> atchan->mask);
>
> tasklet_schedule(>tasklet);
> --
> 2.17.1
>
[PATCH v4 3/3] i2c: at91: added slave mode support
From: Juergen Fitschen
Slave mode driver is based on the concept of i2c-designware driver.
Signed-off-by: Juergen Fitschen
[ludovic.desroc...@microchip.com: rework Kconfig]
Signed-off-by: Ludovic Desroches
---
drivers/i2c/busses/Kconfig | 13 +++
drivers/i2c/busses/Makefile | 3 +
drivers/i2c/busses/i2c-at91-core.c | 13 ++-
drivers/i2c/busses/i2c-at91-slave.c | 147
drivers/i2c/busses/i2c-at91.h | 30 +-
5 files changed, 202 insertions(+), 4 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-at91-slave.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index f2c681971201..6b1f6dcdf533 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -387,6 +387,19 @@ config I2C_AT91
the latency to fill the transmission register is too long. If you
are facing this situation, use the i2c-gpio driver.
+config I2C_AT91_SLAVE_EXPERIMENTAL
+ tristate "Microchip AT91 I2C experimental slave mode"
+ depends on I2C_AT91
+ select I2C_SLAVE
+ help
+ If you say yes to this option, support for the slave mode will be
+ added. Caution: do not use it for production. This feature has not
+ been tested in a heavy way, help wanted.
+ There are known bugs:
+ - It can hang, on a SAMA5D4, after several transfers.
+ - There are some mismtaches with a SAMA5D4 as slave and a SAMA5D2 as
+ master.
+
config I2C_AU1550
tristate "Au1550/Au1200/Au1300 SMBus interface"
depends on MIPS_ALCHEMY
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index ea75a777940e..59b22fbef90a 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -36,6 +36,9 @@ obj-$(CONFIG_I2C_ALTERA) += i2c-altera.o
obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
i2c-at91-objs := i2c-at91-core.o i2c-at91-master.o
+ifeq ($(CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL),y)
+ i2c-at91-objs += i2c-at91-slave.o
+endif
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_AXXIA)+= i2c-axxia.o
obj-$(CONFIG_I2C_BCM2835) += i2c-bcm2835.o
diff --git a/drivers/i2c/busses/i2c-at91-core.c
b/drivers/i2c/busses/i2c-at91-core.c
index 5d9c6c81e6ab..c74283fa567f 100644
--- a/drivers/i2c/busses/i2c-at91-core.c
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -60,8 +60,10 @@ void at91_init_twi_bus(struct at91_twi_dev *dev)
{
at91_disable_twi_interrupts(dev);
at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
-
- at91_init_twi_bus_master(dev);
+ if (dev->slave_detected)
+ at91_init_twi_bus_slave(dev);
+ else
+ at91_init_twi_bus_master(dev);
}
static struct at91_twi_pdata at91rm9200_config = {
@@ -243,7 +245,12 @@ static int at91_twi_probe(struct platform_device *pdev)
dev->adapter.timeout = AT91_I2C_TIMEOUT;
dev->adapter.dev.of_node = pdev->dev.of_node;
- rc = at91_twi_probe_master(pdev, phy_addr, dev);
+ dev->slave_detected = i2c_detect_slave_mode(>dev);
+
+ if (dev->slave_detected)
+ rc = at91_twi_probe_slave(pdev, phy_addr, dev);
+ else
+ rc = at91_twi_probe_master(pdev, phy_addr, dev);
if (rc)
return rc;
diff --git a/drivers/i2c/busses/i2c-at91-slave.c
b/drivers/i2c/busses/i2c-at91-slave.c
new file mode 100644
index ..4b4808e0c8f7
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91-slave.c
@@ -0,0 +1,147 @@
+/*
+ * i2c slave support for Atmel's AT91 Two-Wire Interface (TWI)
+ *
+ * Copyright (C) 2017 Juergen Fitschen
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "i2c-at91.h"
+
+static irqreturn_t atmel_twi_interrupt_slave(int irq, void *dev_id)
+{
+ struct at91_twi_dev *dev = dev_id;
+ const unsigned status = at91_twi_read(dev, AT91_TWI_SR);
+ const unsigned irqstatus = status & at91_twi_read(dev, AT91_TWI_IMR);
+ u8 value;
+
+ if (!irqstatus)
+ return IRQ_NONE;
+
+ /* slave address has been detected on I2C bus */
+ if (irqstatus & AT91_TWI_SVACC) {
+ if (status & AT91_TWI_SVREAD) {
+ i2c_slave_event(dev->slave,
+ I2C_SLAVE_READ_REQUESTED, );
+ writeb_relaxed(value, dev->base + AT91_TWI_THR);
+ at91_twi_write(dev, AT91_TWI_IER,
+ AT91_TWI_TXRDY | AT91_TWI_EOSACC);
+
[PATCH v4 2/3] i2c: at91: split driver into core and master file
From: Juergen Fitschen
The single file i2c-at91.c has been split into core code (i2c-at91-core.c)
and master mode specific code (i2c-at91-master.c). This should enhance
maintainability and reduce ifdeffery for slave mode related code.
The code itself hasn't been touched. Shared functions only had to be made
non-static. Furthermore, includes have been cleaned up.
Signed-off-by: Juergen Fitschen
[ludovic.desroc...@microchip.com: fix checkpatch errors]
Signed-off-by: Ludovic Desroches
---
MAINTAINERS | 3 +-
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-at91-core.c| 373 ++
.../busses/{i2c-at91.c => i2c-at91-master.c} | 467 +-
drivers/i2c/busses/i2c-at91.h | 151 ++
5 files changed, 531 insertions(+), 464 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-at91-core.c
rename drivers/i2c/busses/{i2c-at91.c => i2c-at91-master.c} (67%)
create mode 100644 drivers/i2c/busses/i2c-at91.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 82dc44b09a7e..d61941db6933 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9873,7 +9873,8 @@ MICROCHIP I2C DRIVER
M: Ludovic Desroches
L: linux-...@vger.kernel.org
S: Supported
-F: drivers/i2c/busses/i2c-at91.c
+F: drivers/i2c/busses/i2c-at91.h
+F: drivers/i2c/busses/i2c-at91-*.c
MICROCHIP ISC DRIVER
M: Eugen Hristev
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 5f0cb6915969..ea75a777940e 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_I2C_POWERMAC)+= i2c-powermac.o
obj-$(CONFIG_I2C_ALTERA) += i2c-altera.o
obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
+i2c-at91-objs := i2c-at91-core.o i2c-at91-master.o
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_AXXIA)+= i2c-axxia.o
obj-$(CONFIG_I2C_BCM2835) += i2c-bcm2835.o
diff --git a/drivers/i2c/busses/i2c-at91-core.c
b/drivers/i2c/busses/i2c-at91-core.c
new file mode 100644
index ..5d9c6c81e6ab
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -0,0 +1,373 @@
+/*
+ * i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
+ *
+ * Copyright (C) 2011 Weinmann Medical GmbH
+ * Author: Nikolaus Voss
+ *
+ * Evolved from original work by:
+ * Copyright (C) 2004 Rick Bronson
+ * Converted to 2.6 by Andrew Victor
+ *
+ * Borrowed heavily from original work by:
+ * Copyright (C) 2000 Philip Edelbrock
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "i2c-at91.h"
+
+unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg)
+{
+ return readl_relaxed(dev->base + reg);
+}
+
+void at91_twi_write(struct at91_twi_dev *dev, unsigned reg, unsigned val)
+{
+ writel_relaxed(val, dev->base + reg);
+}
+
+void at91_disable_twi_interrupts(struct at91_twi_dev *dev)
+{
+ at91_twi_write(dev, AT91_TWI_IDR, AT91_TWI_INT_MASK);
+}
+
+void at91_twi_irq_save(struct at91_twi_dev *dev)
+{
+ dev->imr = at91_twi_read(dev, AT91_TWI_IMR) & AT91_TWI_INT_MASK;
+ at91_disable_twi_interrupts(dev);
+}
+
+void at91_twi_irq_restore(struct at91_twi_dev *dev)
+{
+ at91_twi_write(dev, AT91_TWI_IER, dev->imr);
+}
+
+void at91_init_twi_bus(struct at91_twi_dev *dev)
+{
+ at91_disable_twi_interrupts(dev);
+ at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
+
+ at91_init_twi_bus_master(dev);
+}
+
+static struct at91_twi_pdata at91rm9200_config = {
+ .clk_max_div = 5,
+ .clk_offset = 3,
+ .has_unre_flag = true,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9261_config = {
+ .clk_max_div = 5,
+ .clk_offset = 4,
+ .has_unre_flag = false,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9260_config = {
+ .clk_max_div = 7,
+ .clk_offset = 4,
+ .has_unre_flag = false,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9g20_config = {
+ .clk_max_div = 7,
+ .clk_offset = 4,
+ .has_unre_flag = false,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static struct at91_twi_pdata at91sam9g10_config = {
+ .clk_max_div = 7,
+ .clk_offset = 4,
+ .has_unre_flag = false,
+ .has_alt_cmd = false,
+ .has_hold_field = false,
+};
+
+static const struct platform_device_id at91_twi_devtypes[] = {
[PATCH v4 1/3] i2c: at91: segregate master mode specific code from probe and init func
From: Juergen Fitschen
In order to implement slave mode support for the at91 hardware we have to
segregate all master mode specific function parts from the general parts.
The upcoming slave mode patch will call its sepcific probe resp. init
function instead of the master mode functions after the shared general
code has been executed.
This concept has been influenced by the i2c-designware driver.
Signed-off-by: Juergen Fitschen
Signed-off-by: Ludovic Desroches
---
drivers/i2c/busses/i2c-at91.c | 90 +--
1 file changed, 55 insertions(+), 35 deletions(-)
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 3f3e8b3bf5ff..bc74184462c6 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -174,10 +174,8 @@ static void at91_twi_irq_restore(struct at91_twi_dev *dev)
at91_twi_write(dev, AT91_TWI_IER, dev->imr);
}
-static void at91_init_twi_bus(struct at91_twi_dev *dev)
+static void at91_init_twi_bus_master(struct at91_twi_dev *dev)
{
- at91_disable_twi_interrupts(dev);
- at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
/* FIFO should be enabled immediately after the software reset */
if (dev->fifo_size)
at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_FIFOEN);
@@ -186,6 +184,14 @@ static void at91_init_twi_bus(struct at91_twi_dev *dev)
at91_twi_write(dev, AT91_TWI_CWGR, dev->twi_cwgr_reg);
}
+static void at91_init_twi_bus(struct at91_twi_dev *dev)
+{
+ at91_disable_twi_interrupts(dev);
+ at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
+
+ at91_init_twi_bus_master(dev);
+}
+
/*
* Calculate symmetric clock as stated in datasheet:
* twi_clk = F_MAIN / (2 * (cdiv * (1 << ckdiv) + offset))
@@ -1046,18 +1052,56 @@ static struct at91_twi_pdata *at91_twi_get_driver_data(
return (struct at91_twi_pdata *)
platform_get_device_id(pdev)->driver_data;
}
+static int at91_twi_probe_master(struct platform_device *pdev,
+u32 phy_addr, struct at91_twi_dev *dev)
+{
+ int rc;
+ u32 bus_clk_rate;
+
+ init_completion(>cmd_complete);
+
+ rc = devm_request_irq(>dev, dev->irq, atmel_twi_interrupt, 0,
+ dev_name(dev->dev), dev);
+ if (rc) {
+ dev_err(dev->dev, "Cannot get irq %d: %d\n", dev->irq, rc);
+ return rc;
+ }
+
+ if (dev->dev->of_node) {
+ rc = at91_twi_configure_dma(dev, phy_addr);
+ if (rc == -EPROBE_DEFER)
+ return rc;
+ }
+
+ if (!of_property_read_u32(pdev->dev.of_node, "atmel,fifo-size",
+ >fifo_size)) {
+ dev_info(dev->dev, "Using FIFO (%u data)\n", dev->fifo_size);
+ }
+
+ rc = of_property_read_u32(dev->dev->of_node, "clock-frequency",
+ _clk_rate);
+ if (rc)
+ bus_clk_rate = DEFAULT_TWI_CLK_HZ;
+
+ at91_calc_twi_clock(dev, bus_clk_rate);
+
+ dev->adapter.algo = _twi_algorithm;
+ dev->adapter.quirks = _twi_quirks;
+
+ return 0;
+}
+
static int at91_twi_probe(struct platform_device *pdev)
{
struct at91_twi_dev *dev;
struct resource *mem;
int rc;
u32 phy_addr;
- u32 bus_clk_rate;
dev = devm_kzalloc(>dev, sizeof(*dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
- init_completion(>cmd_complete);
+
dev->dev = >dev;
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1077,13 +1121,6 @@ static int at91_twi_probe(struct platform_device *pdev)
if (dev->irq < 0)
return dev->irq;
- rc = devm_request_irq(>dev, dev->irq, atmel_twi_interrupt, 0,
-dev_name(dev->dev), dev);
- if (rc) {
- dev_err(dev->dev, "Cannot get irq %d: %d\n", dev->irq, rc);
- return rc;
- }
-
platform_set_drvdata(pdev, dev);
dev->clk = devm_clk_get(dev->dev, NULL);
@@ -1095,38 +1132,21 @@ static int at91_twi_probe(struct platform_device *pdev)
if (rc)
return rc;
- if (dev->dev->of_node) {
- rc = at91_twi_configure_dma(dev, phy_addr);
- if (rc == -EPROBE_DEFER) {
- clk_disable_unprepare(dev->clk);
- return rc;
- }
- }
-
- if (!of_property_read_u32(pdev->dev.of_node, "atmel,fifo-size",
- >fifo_size)) {
- dev_info(dev->dev, "Using FIFO (%u data)\n", dev->fifo_size);
- }
-
- rc = of_property_read_u32(dev->dev->of_node, "clock-frequency",
-
[PATCH v4 0/3] i2c: at91: slave mode support
[Ludovic Desroches: see Changes section]
Based on the discussion we had on the i2c-linux list [1], I wrote a patch for
AT91 hardware and tried to fulfill the Linux I2C slave interface description
[2] as good as possible. This enables aforementioned hardware to act as an I2C
slave that can be accessed by a remote I2C master.
I have tested this patchset successfully on an ATSAMA5D27.
^ 3.3V ^ 3.3V
+---+| | +---+
| Slave: ATSAMA5D27 | +-+ +-+| Master: ATSAMA5D35|
| with i2c-slave-eeprom | | | 100k | | 100k | with i2cset |
+---+-+-+ +-+ +-++-+-+---+
| | | | | |
| +--+-|---(SDA)---+ |
+--+---(SCL)-+
Schematic: Connection of slave and master with 100kOhm pullup resistors.
On the master the following BASH script has been used to stress the slave.
root@emblinux:~# cat ./stress.sh
#!/bin/bash
I=0
while true
do
if i2cset -y -r 1 0x64 0 $I w | grep mismatch
then
echo "$(date): Error in transmission ${I}"
fi
((I++))
if [ $I -eq 65536 ]
then
I=0
echo "$(date): Sent 65536 transmissions"
fi
done
After running the script for some time I had the following output. To me this
looks promising :)
root@emblinux:~# ./stress.sh
Thu Nov 9 13:58:45 CTE 2017: Sent 65536 transmissions
Thu Nov 9 14:35:20 CTE 2017: Sent 65536 transmissions
Thu Nov 9 15:12:11 CTE 2017: Sent 65536 transmissions
Thu Nov 9 15:49:04 CTE 2017: Sent 65536 transmissions
Thu Nov 9 16:26:00 CTE 2017: Sent 65536 transmissions
Thu Nov 9 17:03:07 UTC 2017: Sent 65536 transmissions
Thu Nov 9 17:40:15 UTC 2017: Sent 65536 transmissions
If you have some hardware with an at91-i2c interface included at hand,
I really
would appreciate if you can run the test script on your hardware and
test this
driver.
Thu Nov 9 17:40:15 UTC 2017: Sent 65536 transmissions
If you have some hardware with an at91-i2c interface included at hand,
I really
would appreciate if you can run the test script on your hardware and
test this
driver.
Best regards
Juergen
[Ludovic Desroches]
Changes in v4:
- rebased on next-20181224
- update Kconfig part to state that this feature is experimental.
- tested quickly on a single board, SAMA5D2 Xplained, i2c-gpio as master, OK.
Changes in v3:
- rebase (cherry-pick was enough)
- fix checkpatch errors
- tests:
- hangs with a SAMA5D4 (master and slave on different busses) after about
100 transfers. It's the first time I do this test.
- some mismatches with a SAMA5D4 as slave and a SAMA5D2 as master
I don't know if it's a regression. I don't remember having seen this
behavior previously.
I think it's worth taking those patches even if there are some possible
bugs. It'll allow to get more people using it and so to consolidate the
slave mode support.
Changes in v2:
- Implemented all suggestions made by Ludovic. (Thank you!)
- Reworked the IRQ handler completely. Have a look in patch 3 fort further
details.
[1] https://marc.info/?t=15082400481=1=1
[2] https://www.kernel.org/doc/Documentation/i2c/slave-interface
Juergen Fitschen (3):
i2c: at91: segregate master mode specific code from probe and init
func
i2c: at91: split driver into core and master file
i2c: at91: added slave mode support
MAINTAINERS | 3 +-
drivers/i2c/busses/Kconfig| 13 +
drivers/i2c/busses/Makefile | 4 +
drivers/i2c/busses/i2c-at91-core.c| 380 +++
.../busses/{i2c-at91.c => i2c-at91-master.c} | 453 +-
drivers/i2c/busses/i2c-at91-slave.c | 147 ++
drivers/i2c/busses/i2c-at91.h | 179 +++
7 files changed, 732 insertions(+), 447 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-at91-core.c
rename drivers/i2c/busses/{i2c-at91.c => i2c-at91-master.c} (67%)
create mode 100644 drivers/i2c/busses/i2c-at91-slave.c
create mode 100644 drivers/i2c/busses/i2c-at91.h
--
2.20.1
Re: [PATCH 07/20] dmaengine: at_hdmac: drop useless LIST_HEAD
On Sun, Dec 23, 2018 at 09:57:02AM +0100, Julia Lawall wrote:
> Drop LIST_HEAD where the variable it declares is never used.
>
> tmp_list has been declared since the introduction of the driver
> and has never been used. The two declarations of list were
> introduced with the containing functions but were also not used.
>
> The semantic patch that fixes this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> //
> @@
> identifier x;
> @@
> - LIST_HEAD(x);
> ... when != x
> //
>
> Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA
> Controller")
> Fixes: 4facfe7f09f2b ("dmaengine: hdmac: Split device_control")
> Signed-off-by: Julia Lawall
Acked-by: Ludovic Desroches
Thanks
>
> ---
> Successfully 0-day tested on 151 configurations.
>
> drivers/dma/at_hdmac.c |5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 01d936c9fe89..a0a9cd76c1d4 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -134,7 +134,6 @@ static struct at_desc *atc_desc_get(struct at_dma_chan
> *atchan)
> struct at_desc *ret = NULL;
> unsigned long flags;
> unsigned int i = 0;
> - LIST_HEAD(tmp_list);
>
> spin_lock_irqsave(>lock, flags);
> list_for_each_entry_safe(desc, _desc, >free_list, desc_node) {
> @@ -1387,8 +1386,6 @@ static int atc_pause(struct dma_chan *chan)
> int chan_id = atchan->chan_common.chan_id;
> unsigned long flags;
>
> - LIST_HEAD(list);
> -
> dev_vdbg(chan2dev(chan), "%s\n", __func__);
>
> spin_lock_irqsave(>lock, flags);
> @@ -1408,8 +1405,6 @@ static int atc_resume(struct dma_chan *chan)
> int chan_id = atchan->chan_common.chan_id;
> unsigned long flags;
>
> - LIST_HEAD(list);
> -
> dev_vdbg(chan2dev(chan), "%s\n", __func__);
>
> if (!atc_chan_is_paused(atchan))
>
Re: [PATCH v3 0/3] i2c: at91: slave mode support
Hi Wolfram, On Tue, Dec 11, 2018 at 08:28:05PM +0100, Wolfram Sang wrote: > Hi Ludovic, > > > > > - fix checkpatch errors > > > > - tests: > > > >- hangs with a SAMA5D4 (master and slave on different busses) after > > > > about > > > >100 transfers. It's the firs time I do this test. > > > >- some mismatches with a SAMA5D4 as slave and a SAMA5D2 as master > > > >I don't know if it's a regression. I don't remember having seen this > > > >behavior previously. > > > >I think it's worth taking those patches even if there are some > > > > possible > > > >bugs. It'll allow to get more people using it and so to consolidate > > > > the > > > >slave mode support. > > > > > > I really want to see those patches go upstream, too. But I am also not a > > > big fan of delivering the user something with known issues. Especially > > > not when they affect the main feature to be added. My rationale here is > > > that someone who is able to fix the issues remaining will also be able > > > to pick up and apply patches. > > > > > > Maybe, maybe if it was to be enabled by a special > > > I2C_AT91_SLAVE_EXPERIMANTEL symbol with lots of explanations. I need to > > > think twice about that, though. > > > > > > > I understand your point. The experimental mentionning could be a good > > trade-off. Let me know once you make up your mind. > > > > > Speaking of Kconfig, I think this series needs to place a > > > > > > select I2C_SLAVE > > > > > > somewhere. > > > > > > > Ok I'll update it if we go further with this set of patches. > > Ok, I give in. If you: > > * add 'select I2C_SLAVE' > * make slave support selectable by I2C_AT91_SLAVE_STAGING or > _EXPERIMENTAL or something alike (default n) > * and add to the help text of that symbol the above known issues > and 'not for production' and 'help wanted' and where to get more > info and all that > > then I'll apply this series soonish. Promised! Ok I will handle these requests and resend it. Thanks Ludovic
Re: [PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes
On Tue, Dec 11, 2018 at 03:35:45PM +0100, Alexandre Belloni wrote:
> On 11/12/2018 12:32:40+, tudor.amba...@microchip.com wrote:
> > Hi, Alexandre,
> >
> > On 12/10/2018 11:35 PM, Alexandre Belloni wrote:
> > > Hi,
> > >
> > > On 10/12/2018 17:15:29+, tudor.amba...@microchip.com wrote:
> > >> From: Cyrille Pitchen
> > >>
> > >> This patch configures the QSPI0 controller pin muxing and declares
> > >> a jedec,spi-nor memory.
> > >>
> > >> sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
> > >> memory which advertises a maximum frequency of 80MHz for Quad IO
> > >> Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
> > >> the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.
> > >>
> > >> Signed-off-by: Cyrille Pitchen
> > >> [tudor.amba...@microchip.com:
> > >> - drop partitions,
> > >> - add spi-rx/tx-bus-width
> > >> - change spi-max-frequency to match the 80MHz limit advertised by
> > >> MX25L25673G for Quad IO Fast Read,
> > >> - reword commit message and subject.]
> > >> Signed-off-by: Tudor Ambarus
> > >> ---
> > >> arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31
> > >> +
> > >> 1 file changed, 31 insertions(+)
> > >>
> > >> diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > >> b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > >> index 518e2b095ccf..171bc82cfbbf 100644
> > >> --- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > >> +++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > >> @@ -108,6 +108,21 @@
> > >> };
> > >>
> > >> apb {
> > >> +qspi0: spi@f002 {
> > >> +pinctrl-names = "default";
> > >> +pinctrl-0 = <_qspi0_default>;
> > >> +/* status = "okay"; */ /* conflict with
> > >> sdmmc1 */
> > >
> > > Isn't that conflicting then because I think the default is okay.
> > qspi0 is disabled in sama5d2.dtsi.
> >
>
> Ok, then maybe that comment is not necessary at all.
Usually we do it the other way around:
status = "disabled"; /* conflict with ... */
Regards
Ludovic
Re: [PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes
On Tue, Dec 11, 2018 at 03:40:33PM +0100, Boris Brezillon wrote:
> On Mon, 10 Dec 2018 17:15:29 +
> wrote:
>
> > From: Cyrille Pitchen
> >
> > This patch configures the QSPI0 controller pin muxing and declares
> > a jedec,spi-nor memory.
> >
> > sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
> > memory which advertises a maximum frequency of 80MHz for Quad IO
> > Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
> > the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.
> >
> > Signed-off-by: Cyrille Pitchen
> > [tudor.amba...@microchip.com:
> > - drop partitions,
> > - add spi-rx/tx-bus-width
> > - change spi-max-frequency to match the 80MHz limit advertised by
> > MX25L25673G for Quad IO Fast Read,
> > - reword commit message and subject.]
> > Signed-off-by: Tudor Ambarus
> > ---
> > arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31
> > +
> > 1 file changed, 31 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > index 518e2b095ccf..171bc82cfbbf 100644
> > --- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > +++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > @@ -108,6 +108,21 @@
> > };
> >
> > apb {
> > + qspi0: spi@f002 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <_qspi0_default>;
> > + /* status = "okay"; */ /* conflict with sdmmc1
> > */
> > +
> > + flash@0 {
> > + compatible = "jedec,spi-nor";
> > + reg = <0>;
> > + spi-max-frequency = <8000>;
> > + spi-tx-bus-width = <4>;
> > + spi-rx-bus-width = <4>;
> > + m25p,fast-read;
> > + };
>
> I'm a bit lost. What's the point of defining this if the QSPI
> controller is not enabled?
It's a way to avoid customer struggling with the device tree. If he
doesn't care about sdmmc1, he can easily enable the qpsi controller and
get access to the memory.
Regards
Ludovic
Re: [PATCH] dmaengine: at_hdmac: fix module unloading
On Tue, Nov 27, 2018 at 05:06:35PM +0100, Richard Genoud wrote:
> of_dma_controller_free() was not called on module onloading.
s/onloading/unloading
> This lead to a soft lockup:
> watchdog: BUG: soft lockup - CPU#0 stuck for 23s!
> Modules linked in: at_hdmac [last unloaded: at_hdmac]
> when of_dma_request_slave_channel() tried to call ofdma->of_dma_xlate().
>
> Cc: sta...@vger.kernel.org
> Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
> Signed-off-by: Richard Genoud
Acked-by: Ludovic Desroches
Thanks
Ludovic
> ---
> drivers/dma/at_hdmac.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 1b7f0ca0d5cd..01d936c9fe89 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -2006,6 +2006,8 @@ static int at_dma_remove(struct platform_device *pdev)
> struct resource *io;
>
> at_dma_off(atdma);
> + if (pdev->dev.of_node)
> + of_dma_controller_free(pdev->dev.of_node);
> dma_async_device_unregister(>dma_common);
>
> dma_pool_destroy(atdma->memset_pool);
Re: [PATCH] dmaengine: at_hdmac: fix module unloading
On Tue, Nov 27, 2018 at 05:06:35PM +0100, Richard Genoud wrote:
> of_dma_controller_free() was not called on module onloading.
s/onloading/unloading
> This lead to a soft lockup:
> watchdog: BUG: soft lockup - CPU#0 stuck for 23s!
> Modules linked in: at_hdmac [last unloaded: at_hdmac]
> when of_dma_request_slave_channel() tried to call ofdma->of_dma_xlate().
>
> Cc: sta...@vger.kernel.org
> Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
> Signed-off-by: Richard Genoud
Acked-by: Ludovic Desroches
Thanks
Ludovic
> ---
> drivers/dma/at_hdmac.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 1b7f0ca0d5cd..01d936c9fe89 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -2006,6 +2006,8 @@ static int at_dma_remove(struct platform_device *pdev)
> struct resource *io;
>
> at_dma_off(atdma);
> + if (pdev->dev.of_node)
> + of_dma_controller_free(pdev->dev.of_node);
> dma_async_device_unregister(>dma_common);
>
> dma_pool_destroy(atdma->memset_pool);
Re: [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate()
On Tue, Nov 27, 2018 at 05:06:34PM +0100, Richard Genoud wrote:
> The leak was found when opening/closing a serial port a great number of
> time, increasing kmalloc-32 in slabinfo.
>
> Each time the port was opened, dma_request_slave_channel() was called.
> Then, in at_dma_xlate(), atslave was allocated with devm_kzalloc() and
> never freed. (Well, it was free at module unload, but that's not what we
> want).
> So, here, kzalloc is more suited for the job since it has to be freed in
> atc_free_chan_resources().
>
> Cc: sta...@vger.kernel.org
> Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
> Reported-by: Mario Forner
> Suggested-by: Alexandre Belloni
> Acked-by: Alexandre Belloni
> Signed-off-by: Richard Genoud
Acked-by: Ludovic Desroches
Thanks for handling this issue.
Ludovic
> ---
> drivers/dma/at_hdmac.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 7cbac6e8c113..1b7f0ca0d5cd 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -1641,6 +1641,12 @@ static void atc_free_chan_resources(struct dma_chan
> *chan)
> atchan->descs_allocated = 0;
> atchan->status = 0;
>
> + /*
> + * Free atslave allocated in at_dma_xlate()
> + */
> + kfree(chan->private);
> + chan->private = NULL;
> +
> dev_vdbg(chan2dev(chan), "free_chan_resources: done\n");
> }
>
> @@ -1675,7 +1681,7 @@ static struct dma_chan *at_dma_xlate(struct
> of_phandle_args *dma_spec,
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
>
> - atslave = devm_kzalloc(_pdev->dev, sizeof(*atslave), GFP_KERNEL);
> + atslave = kzalloc(sizeof(*atslave), GFP_KERNEL);
> if (!atslave)
> return NULL;
>
Re: [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate()
On Tue, Nov 27, 2018 at 05:06:34PM +0100, Richard Genoud wrote:
> The leak was found when opening/closing a serial port a great number of
> time, increasing kmalloc-32 in slabinfo.
>
> Each time the port was opened, dma_request_slave_channel() was called.
> Then, in at_dma_xlate(), atslave was allocated with devm_kzalloc() and
> never freed. (Well, it was free at module unload, but that's not what we
> want).
> So, here, kzalloc is more suited for the job since it has to be freed in
> atc_free_chan_resources().
>
> Cc: sta...@vger.kernel.org
> Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
> Reported-by: Mario Forner
> Suggested-by: Alexandre Belloni
> Acked-by: Alexandre Belloni
> Signed-off-by: Richard Genoud
Acked-by: Ludovic Desroches
Thanks for handling this issue.
Ludovic
> ---
> drivers/dma/at_hdmac.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 7cbac6e8c113..1b7f0ca0d5cd 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -1641,6 +1641,12 @@ static void atc_free_chan_resources(struct dma_chan
> *chan)
> atchan->descs_allocated = 0;
> atchan->status = 0;
>
> + /*
> + * Free atslave allocated in at_dma_xlate()
> + */
> + kfree(chan->private);
> + chan->private = NULL;
> +
> dev_vdbg(chan2dev(chan), "free_chan_resources: done\n");
> }
>
> @@ -1675,7 +1681,7 @@ static struct dma_chan *at_dma_xlate(struct
> of_phandle_args *dma_spec,
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
>
> - atslave = devm_kzalloc(_pdev->dev, sizeof(*atslave), GFP_KERNEL);
> + atslave = kzalloc(sizeof(*atslave), GFP_KERNEL);
> if (!atslave)
> return NULL;
>
Re: [PATCH v3 2/7] ARM: dts: at91: sama5d2: switch to new binding
clock is missing in the title of this patch. Regards Ludovic On Mon, Nov 12, 2018 at 02:31:03PM +0100, Alexandre Belloni wrote: > Switch sama5d2 boards to the new PMC clock bindings. > > Signed-off-by: Alexandre Belloni > --- > arch/arm/boot/dts/at91-sama5d27_som1_ek.dts | 12 +- > arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts | 2 +- > arch/arm/boot/dts/at91-sama5d2_xplained.dts | 4 +- > arch/arm/boot/dts/sama5d2.dtsi | 670 ++-- > 4 files changed, 69 insertions(+), 619 deletions(-) > > diff --git a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts > b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts > index 363a43d77424..4a258867ddf1 100644 > --- a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts > +++ b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts > @@ -165,7 +165,7 @@ > dma-names = "tx", "rx"; > #address-cells = <1>; > #size-cells = <0>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 20>; > pinctrl-names = "default"; > pinctrl-0 = <_mikrobus_i2c>; > atmel,fifo-size = <16>; > @@ -211,7 +211,7 @@ > compatible = "atmel,at91sam9260-usart"; > reg = <0x200 0x200>; > interrupts = <22 IRQ_TYPE_LEVEL_HIGH 7>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 22>; > clock-names = "usart"; > pinctrl-names = "default"; > pinctrl-0 = <_flx3_default>; > @@ -223,7 +223,7 @@ > compatible = "atmel,at91rm9200-spi"; > reg = <0x400 0x200>; > interrupts = <22 IRQ_TYPE_LEVEL_HIGH 7>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 22>; > clock-names = "spi_clk"; > pinctrl-names = "default"; > pinctrl-0 = <_flx3_default>; > @@ -240,7 +240,7 @@ > compatible = "atmel,at91sam9260-usart"; > reg = <0x200 0x200>; > interrupts = <23 IRQ_TYPE_LEVEL_HIGH 7>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 23>; > clock-names = "usart"; > pinctrl-names = "default"; > pinctrl-0 = <_flx4_default>; > @@ -252,7 +252,7 @@ > compatible = "atmel,at91rm9200-spi"; > reg = <0x400 0x200>; > interrupts = <23 IRQ_TYPE_LEVEL_HIGH 7>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 23>; > clock-names = "spi_clk"; > pinctrl-names = "default"; > pinctrl-0 = <_mikrobus_spi > _mikrobus1_spi_cs _mikrobus2_spi_cs>; > @@ -268,7 +268,7 @@ > dma-names = "tx", "rx"; > #address-cells = <1>; > #size-cells = <0>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 23>; > pinctrl-names = "default"; > pinctrl-0 = <_flx4_default>; > atmel,fifo-size = <16>; > diff --git a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts > b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts > index 2214bfe7aa20..ba7f3e646c26 100644 > --- a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts > +++ b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts > @@ -197,7 +197,7 @@ > dma-names = "tx", "rx"; > #address-cells = <1>; > #size-cells = <0>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 19>; > pinctrl-names = "default"; > pinctrl-0 = <_flx0_default>; > atmel,fifo-size = <16>; > diff
Re: [PATCH v3 2/7] ARM: dts: at91: sama5d2: switch to new binding
clock is missing in the title of this patch. Regards Ludovic On Mon, Nov 12, 2018 at 02:31:03PM +0100, Alexandre Belloni wrote: > Switch sama5d2 boards to the new PMC clock bindings. > > Signed-off-by: Alexandre Belloni > --- > arch/arm/boot/dts/at91-sama5d27_som1_ek.dts | 12 +- > arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts | 2 +- > arch/arm/boot/dts/at91-sama5d2_xplained.dts | 4 +- > arch/arm/boot/dts/sama5d2.dtsi | 670 ++-- > 4 files changed, 69 insertions(+), 619 deletions(-) > > diff --git a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts > b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts > index 363a43d77424..4a258867ddf1 100644 > --- a/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts > +++ b/arch/arm/boot/dts/at91-sama5d27_som1_ek.dts > @@ -165,7 +165,7 @@ > dma-names = "tx", "rx"; > #address-cells = <1>; > #size-cells = <0>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 20>; > pinctrl-names = "default"; > pinctrl-0 = <_mikrobus_i2c>; > atmel,fifo-size = <16>; > @@ -211,7 +211,7 @@ > compatible = "atmel,at91sam9260-usart"; > reg = <0x200 0x200>; > interrupts = <22 IRQ_TYPE_LEVEL_HIGH 7>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 22>; > clock-names = "usart"; > pinctrl-names = "default"; > pinctrl-0 = <_flx3_default>; > @@ -223,7 +223,7 @@ > compatible = "atmel,at91rm9200-spi"; > reg = <0x400 0x200>; > interrupts = <22 IRQ_TYPE_LEVEL_HIGH 7>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 22>; > clock-names = "spi_clk"; > pinctrl-names = "default"; > pinctrl-0 = <_flx3_default>; > @@ -240,7 +240,7 @@ > compatible = "atmel,at91sam9260-usart"; > reg = <0x200 0x200>; > interrupts = <23 IRQ_TYPE_LEVEL_HIGH 7>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 23>; > clock-names = "usart"; > pinctrl-names = "default"; > pinctrl-0 = <_flx4_default>; > @@ -252,7 +252,7 @@ > compatible = "atmel,at91rm9200-spi"; > reg = <0x400 0x200>; > interrupts = <23 IRQ_TYPE_LEVEL_HIGH 7>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 23>; > clock-names = "spi_clk"; > pinctrl-names = "default"; > pinctrl-0 = <_mikrobus_spi > _mikrobus1_spi_cs _mikrobus2_spi_cs>; > @@ -268,7 +268,7 @@ > dma-names = "tx", "rx"; > #address-cells = <1>; > #size-cells = <0>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 23>; > pinctrl-names = "default"; > pinctrl-0 = <_flx4_default>; > atmel,fifo-size = <16>; > diff --git a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts > b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts > index 2214bfe7aa20..ba7f3e646c26 100644 > --- a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts > +++ b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts > @@ -197,7 +197,7 @@ > dma-names = "tx", "rx"; > #address-cells = <1>; > #size-cells = <0>; > - clocks = <_clk>; > + clocks = < PMC_TYPE_PERIPHERAL 19>; > pinctrl-names = "default"; > pinctrl-0 = <_flx0_default>; > atmel,fifo-size = <16>; > diff
Re: [PATCH] i2c: at91: switched to resume/suspend callbacks.
On Mon, Oct 22, 2018 at 12:17:47PM +0200, Andrei Stefanescu - M50506 wrote:
> In the previous version of the driver resume/suspend_noirq callbacks
> were used. Because of this, when resuming from suspend-to-ram,
> an I2C (belonging to a FLEXCOM) would resume before FLEXCOM.
> The first read on the I2C bus would then result in a timeout.
>
> This patch switches to resume/suspend callbacks which are
> called after FLEXCOM resumes. FLEXCOM, SPI and USART drivers use
> resume/suspend callbacks.
>
> Signed-off-by: Andrei Stefanescu
I can't figure out why we use the _noirq variant. When patches for PM
stuff were sent, suspend/resume callbacks were used but in the latest
version it moved to the _noirq variant without explanation.
Excepting if someone has an argument to keep the _noirq variant,
Acked-by: Ludovic Desroches
Thanks
Regards
Ludovic
> ---
> drivers/i2c/busses/i2c-at91.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> index bfd1fdf..81f7b94 100644
> --- a/drivers/i2c/busses/i2c-at91.c
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -1174,7 +1174,7 @@ static int at91_twi_runtime_resume(struct device *dev)
> return clk_prepare_enable(twi_dev->clk);
> }
>
> -static int at91_twi_suspend_noirq(struct device *dev)
> +static int at91_twi_suspend(struct device *dev)
> {
> if (!pm_runtime_status_suspended(dev))
> at91_twi_runtime_suspend(dev);
> @@ -1182,7 +1182,7 @@ static int at91_twi_suspend_noirq(struct device *dev)
> return 0;
> }
>
> -static int at91_twi_resume_noirq(struct device *dev)
> +static int at91_twi_resume(struct device *dev)
> {
> struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
> int ret;
> @@ -1202,8 +1202,8 @@ static int at91_twi_resume_noirq(struct device *dev)
> }
>
> static const struct dev_pm_ops at91_twi_pm = {
> - .suspend_noirq = at91_twi_suspend_noirq,
> - .resume_noirq = at91_twi_resume_noirq,
> + .suspend= at91_twi_suspend,
> + .resume = at91_twi_resume,
> .runtime_suspend= at91_twi_runtime_suspend,
> .runtime_resume = at91_twi_runtime_resume,
> };
> --
> 2.7.4
>
Re: [PATCH] i2c: at91: switched to resume/suspend callbacks.
On Mon, Oct 22, 2018 at 12:17:47PM +0200, Andrei Stefanescu - M50506 wrote:
> In the previous version of the driver resume/suspend_noirq callbacks
> were used. Because of this, when resuming from suspend-to-ram,
> an I2C (belonging to a FLEXCOM) would resume before FLEXCOM.
> The first read on the I2C bus would then result in a timeout.
>
> This patch switches to resume/suspend callbacks which are
> called after FLEXCOM resumes. FLEXCOM, SPI and USART drivers use
> resume/suspend callbacks.
>
> Signed-off-by: Andrei Stefanescu
I can't figure out why we use the _noirq variant. When patches for PM
stuff were sent, suspend/resume callbacks were used but in the latest
version it moved to the _noirq variant without explanation.
Excepting if someone has an argument to keep the _noirq variant,
Acked-by: Ludovic Desroches
Thanks
Regards
Ludovic
> ---
> drivers/i2c/busses/i2c-at91.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> index bfd1fdf..81f7b94 100644
> --- a/drivers/i2c/busses/i2c-at91.c
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -1174,7 +1174,7 @@ static int at91_twi_runtime_resume(struct device *dev)
> return clk_prepare_enable(twi_dev->clk);
> }
>
> -static int at91_twi_suspend_noirq(struct device *dev)
> +static int at91_twi_suspend(struct device *dev)
> {
> if (!pm_runtime_status_suspended(dev))
> at91_twi_runtime_suspend(dev);
> @@ -1182,7 +1182,7 @@ static int at91_twi_suspend_noirq(struct device *dev)
> return 0;
> }
>
> -static int at91_twi_resume_noirq(struct device *dev)
> +static int at91_twi_resume(struct device *dev)
> {
> struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
> int ret;
> @@ -1202,8 +1202,8 @@ static int at91_twi_resume_noirq(struct device *dev)
> }
>
> static const struct dev_pm_ops at91_twi_pm = {
> - .suspend_noirq = at91_twi_suspend_noirq,
> - .resume_noirq = at91_twi_resume_noirq,
> + .suspend= at91_twi_suspend,
> + .resume = at91_twi_resume,
> .runtime_suspend= at91_twi_runtime_suspend,
> .runtime_resume = at91_twi_runtime_resume,
> };
> --
> 2.7.4
>
Re: sama5d: using the ebi interface from another driver
Hi Jean-Michel,
On Fri, Nov 02, 2018 at 02:35:26PM +0100, Jean-Michel Hautbois wrote:
> Hi all,
>
> I have a custom board based on a sama5d3 chip. The SoC is connected to
> 2 pef24628 SHDSL transceivers, the first one on ebi@4000 and the
> second one on ebi@5000.
> I tried to write a basic char driver, using request_mem_region and
> ioremap but I can't read or write into the device.
> I have to say that the driver is based on a proprietary one, and
> tested years ago on a PowerPC board.
>
> Then, after looking into deeper details in the datasheet I understand
> it is connected through EBI and it sounds not so easy :D.
>
> I would appreciate some help/pointers on this, as there is (at least,
> I could find) few documentation on how to use it except for NAND
> cases.
>
> I have something like that in my DTS, but not sure this is the correct
> way to do it :
>
> ebi: ebi@1000 {
> pinctrl-0 = <_ebi_nand_addr>;
> pinctrl-names = "default";
> status = "okay";
>
> dsp0: pef24628@1 {
> status = "okay";
> compatible = "intel,pef24628";
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x1 0x0 0x8000>;
> pinctrl-0 = <_dsp_cs1>;
> };
>
> dsp1: pef24628@2 {
> status = "okay";
> compatible = "intel,pef24628";
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x2 0x0 0x8000>;
> pinctrl-0 = <_dsp_cs2>;
> };
>
> nand_controller: nand-controller {
> status = "okay";
>
> nand@3 {
> reg = <0x3 0x0 0x2>;
> atmel,rb = <0>;
> nand-bus-width = <8>;
> nand-ecc-mode = "hw";
> nand-ecc-strength = <4>;
> nand-ecc-step-size = <512>;
> nand-on-flash-bbt;
> label = "atmel_nand";
>
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <1>;
> #size-cells = <1>;
> [...]
> };
> };
> };
>
> The pinctrl for ebi should probably be changed however, I am wondering
> how the (platform ?) driver can access the adress ? Should it parse
> itself the parent, and find range, etc. Or is there an accessor for it
> ?
At first look, your DT seems correct. You have nothing special to do,
the usual platform_get_resource() and devm_ioremap_resource() calls are enough.
The address translation should be done automatically according to ranges
property.
Another hint is to check and specify the signals timings.
Regards
Ludovic
>
> Maybe can I just manually toggle the CS GPIO, and don't try to make
> anything more complex than what it should be ? The driver should not
> be atmel dependant...
>
> Thanks !
> JM
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: sama5d: using the ebi interface from another driver
Hi Jean-Michel,
On Fri, Nov 02, 2018 at 02:35:26PM +0100, Jean-Michel Hautbois wrote:
> Hi all,
>
> I have a custom board based on a sama5d3 chip. The SoC is connected to
> 2 pef24628 SHDSL transceivers, the first one on ebi@4000 and the
> second one on ebi@5000.
> I tried to write a basic char driver, using request_mem_region and
> ioremap but I can't read or write into the device.
> I have to say that the driver is based on a proprietary one, and
> tested years ago on a PowerPC board.
>
> Then, after looking into deeper details in the datasheet I understand
> it is connected through EBI and it sounds not so easy :D.
>
> I would appreciate some help/pointers on this, as there is (at least,
> I could find) few documentation on how to use it except for NAND
> cases.
>
> I have something like that in my DTS, but not sure this is the correct
> way to do it :
>
> ebi: ebi@1000 {
> pinctrl-0 = <_ebi_nand_addr>;
> pinctrl-names = "default";
> status = "okay";
>
> dsp0: pef24628@1 {
> status = "okay";
> compatible = "intel,pef24628";
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x1 0x0 0x8000>;
> pinctrl-0 = <_dsp_cs1>;
> };
>
> dsp1: pef24628@2 {
> status = "okay";
> compatible = "intel,pef24628";
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x2 0x0 0x8000>;
> pinctrl-0 = <_dsp_cs2>;
> };
>
> nand_controller: nand-controller {
> status = "okay";
>
> nand@3 {
> reg = <0x3 0x0 0x2>;
> atmel,rb = <0>;
> nand-bus-width = <8>;
> nand-ecc-mode = "hw";
> nand-ecc-strength = <4>;
> nand-ecc-step-size = <512>;
> nand-on-flash-bbt;
> label = "atmel_nand";
>
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <1>;
> #size-cells = <1>;
> [...]
> };
> };
> };
>
> The pinctrl for ebi should probably be changed however, I am wondering
> how the (platform ?) driver can access the adress ? Should it parse
> itself the parent, and find range, etc. Or is there an accessor for it
> ?
At first look, your DT seems correct. You have nothing special to do,
the usual platform_get_resource() and devm_ioremap_resource() calls are enough.
The address translation should be done automatically according to ranges
property.
Another hint is to check and specify the signals timings.
Regards
Ludovic
>
> Maybe can I just manually toggle the CS GPIO, and don't try to make
> anything more complex than what it should be ? The driver should not
> be atmel dependant...
>
> Thanks !
> JM
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [RESEND PATCH 1/2] i2c: enable buses to save their clock frequency in adapter
On Tue, Oct 23, 2018 at 11:20:39AM +0200, Tudor Ambarus - M18064 wrote:
> The clock-frequency property is not mandatory for the i2c buses. If it's
> not present in the device tree, the buses __usually__ assume it's 100kHZ
> (see altera, at91, axxia, etc.). Broadcom uses a 375kHZ default
> clock-frequency, so the default clock frequency varies from bus to bus.
>
> There are i2c clients that need to know the bus clock frequency in order to
> compute their wake token (see atecc508a i2c client).
>
> The clock-frequency value has to be propagated to the i2c clients, otherwise,
> if they will not find the i2c bus clock frequency in the device tree, they
> will have to make their own assumption of the clock frequency.
>
> Spare the i2c clients of making wrong assumptions of the i2c bus clock
> frequency and enable the buses to save their clock frequency in adapter.
>
> Signed-off-by: Tudor Ambarus
Reviwed-by: Ludovic Desroches
> ---
> include/linux/i2c.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 65b4eaed1d96..f238da204c49 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -677,6 +677,7 @@ struct i2c_adapter {
> struct rt_mutex bus_lock;
> struct rt_mutex mux_lock;
>
> + u32 bus_freq_hz;
> int timeout;/* in jiffies */
> int retries;
> struct device dev; /* the adapter device */
> --
> 2.9.4
>
Re: [RESEND PATCH 1/2] i2c: enable buses to save their clock frequency in adapter
On Tue, Oct 23, 2018 at 11:20:39AM +0200, Tudor Ambarus - M18064 wrote:
> The clock-frequency property is not mandatory for the i2c buses. If it's
> not present in the device tree, the buses __usually__ assume it's 100kHZ
> (see altera, at91, axxia, etc.). Broadcom uses a 375kHZ default
> clock-frequency, so the default clock frequency varies from bus to bus.
>
> There are i2c clients that need to know the bus clock frequency in order to
> compute their wake token (see atecc508a i2c client).
>
> The clock-frequency value has to be propagated to the i2c clients, otherwise,
> if they will not find the i2c bus clock frequency in the device tree, they
> will have to make their own assumption of the clock frequency.
>
> Spare the i2c clients of making wrong assumptions of the i2c bus clock
> frequency and enable the buses to save their clock frequency in adapter.
>
> Signed-off-by: Tudor Ambarus
Reviwed-by: Ludovic Desroches
> ---
> include/linux/i2c.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 65b4eaed1d96..f238da204c49 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -677,6 +677,7 @@ struct i2c_adapter {
> struct rt_mutex bus_lock;
> struct rt_mutex mux_lock;
>
> + u32 bus_freq_hz;
> int timeout;/* in jiffies */
> int retries;
> struct device dev; /* the adapter device */
> --
> 2.9.4
>
[GIT PULL] ARM: at91: DT for 4.20 #2
Arnd, Olof, Here are the second round of DT changes for 4.20. It consists in NAND description fixes (size and partitions). Regards Ludovic The following changes since commit 97181516b0785dd032700ae4899842389c6bea78: arm: dts: sama5d2: Update coresight bindings for hardware ports (2018-09-19 19:12:25 +0200) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/at91/linux tags/at91-4.20-dt2 for you to fetch changes up to 767466b63de3368376323958bb9319bfa8d8: ARM: dts: at91: sama5d4_xplained: even nand memory partitions (2018-10-03 10:26:57 +0200) AT91 DT for 4.20 #2 - fixes NAND size and partitions for some boards Tudor Ambarus (6): ARM: dts: at91: sama5d4_xplained: fix addressable nand flash size ARM: dts: at91: at91sam9x5cm: fix addressable nand flash size ARM: dts: at91: sama5d2_ptc_ek: fix bootloader env offsets ARM: dts: at91: at91sam9x5cm: even nand memory partitions ARM: dts: at91: sama5d3_xplained: even nand memory partitions ARM: dts: at91: sama5d4_xplained: even nand memory partitions arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts | 8 arch/arm/boot/dts/at91-sama5d3_xplained.dts | 11 --- arch/arm/boot/dts/at91-sama5d4_xplained.dts | 13 + arch/arm/boot/dts/at91sam9x5cm.dtsi | 18 ++ 4 files changed, 35 insertions(+), 15 deletions(-)
[GIT PULL] ARM: at91: DT for 4.20 #2
Arnd, Olof, Here are the second round of DT changes for 4.20. It consists in NAND description fixes (size and partitions). Regards Ludovic The following changes since commit 97181516b0785dd032700ae4899842389c6bea78: arm: dts: sama5d2: Update coresight bindings for hardware ports (2018-09-19 19:12:25 +0200) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/at91/linux tags/at91-4.20-dt2 for you to fetch changes up to 767466b63de3368376323958bb9319bfa8d8: ARM: dts: at91: sama5d4_xplained: even nand memory partitions (2018-10-03 10:26:57 +0200) AT91 DT for 4.20 #2 - fixes NAND size and partitions for some boards Tudor Ambarus (6): ARM: dts: at91: sama5d4_xplained: fix addressable nand flash size ARM: dts: at91: at91sam9x5cm: fix addressable nand flash size ARM: dts: at91: sama5d2_ptc_ek: fix bootloader env offsets ARM: dts: at91: at91sam9x5cm: even nand memory partitions ARM: dts: at91: sama5d3_xplained: even nand memory partitions ARM: dts: at91: sama5d4_xplained: even nand memory partitions arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts | 8 arch/arm/boot/dts/at91-sama5d3_xplained.dts | 11 --- arch/arm/boot/dts/at91-sama5d4_xplained.dts | 13 + arch/arm/boot/dts/at91sam9x5cm.dtsi | 18 ++ 4 files changed, 35 insertions(+), 15 deletions(-)
Re: [PATCH 2/2] ARM: dts: at91: at91sam9x5cm: fix addressable nand flash size
On Tue, Oct 02, 2018 at 02:29:49PM +0300, Tudor Ambarus wrote:
> at91sam9x5cm comes with a 2Gb NAND flash. Fix the rootfs size to
> match this limit.
>
> Signed-off-by: Tudor Ambarus
Acked-by: Ludovic Desroches
Thanks
> ---
> arch/arm/boot/dts/at91sam9x5cm.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/at91sam9x5cm.dtsi
> b/arch/arm/boot/dts/at91sam9x5cm.dtsi
> index 4908ee07e628..993eabe1cf7a 100644
> --- a/arch/arm/boot/dts/at91sam9x5cm.dtsi
> +++ b/arch/arm/boot/dts/at91sam9x5cm.dtsi
> @@ -100,7 +100,7 @@
>
> rootfs@80 {
> label = "rootfs";
> - reg = <0x80
> 0x1f80>;
> + reg = <0x80
> 0x0f80>;
> };
> };
> };
> --
> 2.9.4
>
Re: [PATCH 2/2] ARM: dts: at91: at91sam9x5cm: fix addressable nand flash size
On Tue, Oct 02, 2018 at 02:29:49PM +0300, Tudor Ambarus wrote:
> at91sam9x5cm comes with a 2Gb NAND flash. Fix the rootfs size to
> match this limit.
>
> Signed-off-by: Tudor Ambarus
Acked-by: Ludovic Desroches
Thanks
> ---
> arch/arm/boot/dts/at91sam9x5cm.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/at91sam9x5cm.dtsi
> b/arch/arm/boot/dts/at91sam9x5cm.dtsi
> index 4908ee07e628..993eabe1cf7a 100644
> --- a/arch/arm/boot/dts/at91sam9x5cm.dtsi
> +++ b/arch/arm/boot/dts/at91sam9x5cm.dtsi
> @@ -100,7 +100,7 @@
>
> rootfs@80 {
> label = "rootfs";
> - reg = <0x80
> 0x1f80>;
> + reg = <0x80
> 0x0f80>;
> };
> };
> };
> --
> 2.9.4
>
Re: [PATCH 1/2] ARM: dts: at91: sama5d4_xplained: fix addresable nand flash size
On Tue, Oct 02, 2018 at 02:29:48PM +0300, Tudor Ambarus wrote:
> sama5d4_xplained comes with a 4Gb NAND flash. Increase the rootfs
> size to match this limit.
>
> Signed-off-by: Tudor Ambarus
Acked-by: Ludovic Desroches
Thanks
> ---
> arch/arm/boot/dts/at91-sama5d4_xplained.dts | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> index 4b7c762d5f22..7d554b9ab27f 100644
> --- a/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> +++ b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> @@ -252,7 +252,7 @@
>
> rootfs@80 {
> label = "rootfs";
> - reg = <0x80
> 0x0f80>;
> + reg = <0x80
> 0x1f80>;
> };
> };
> };
> --
> 2.9.4
>
Re: [PATCH 1/2] ARM: dts: at91: sama5d4_xplained: fix addresable nand flash size
On Tue, Oct 02, 2018 at 02:29:48PM +0300, Tudor Ambarus wrote:
> sama5d4_xplained comes with a 4Gb NAND flash. Increase the rootfs
> size to match this limit.
>
> Signed-off-by: Tudor Ambarus
Acked-by: Ludovic Desroches
Thanks
> ---
> arch/arm/boot/dts/at91-sama5d4_xplained.dts | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> index 4b7c762d5f22..7d554b9ab27f 100644
> --- a/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> +++ b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
> @@ -252,7 +252,7 @@
>
> rootfs@80 {
> label = "rootfs";
> - reg = <0x80
> 0x0f80>;
> + reg = <0x80
> 0x1f80>;
> };
> };
> };
> --
> 2.9.4
>
[PATCH v5 2/2] tty/serial: atmel: add ISO7816 support
From: Nicolas Ferre
When mode is set in atmel_config_iso7816() we backup last RS232 mode
for coming back to this mode if requested.
Also allow setup of T=0 and T=1 parameter and basic support in set_termios
function as well.
Signed-off-by: Nicolas Ferre
[ludovic.desroc...@microchip.com: rebase, add check on fidi ratio, checkpatch
fixes]
Signed-off-by: Ludovic Desroches
Acked-by: Richard Genoud
---
drivers/tty/serial/atmel_serial.c | 190 +++---
drivers/tty/serial/atmel_serial.h | 3 +-
2 files changed, 181 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c
b/drivers/tty/serial/atmel_serial.c
index 267d4d1de3f8..05147fe24343 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -34,6 +34,7 @@
#include
#include
+#include
#include
#include
@@ -147,6 +148,8 @@ struct atmel_uart_port {
struct circ_buf rx_ring;
struct mctrl_gpios *gpios;
+ u32 backup_mode;/* MR saved during iso7816
operations */
+ u32 backup_brgr;/* BRGR saved during iso7816
operations */
unsigned inttx_done_mask;
u32 fifo_size;
u32 rts_high;
@@ -163,6 +166,10 @@ struct atmel_uart_port {
unsigned intpending_status;
spinlock_t lock_suspended;
+ /* ISO7816 */
+ unsigned intfidi_min;
+ unsigned intfidi_max;
+
#ifdef CONFIG_PM
struct {
u32 cr;
@@ -361,6 +368,127 @@ static int atmel_config_rs485(struct uart_port *port,
return 0;
}
+static unsigned int atmel_calc_cd(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int cd;
+ u64 mck_rate;
+
+ mck_rate = (u64)clk_get_rate(atmel_port->clk);
+ do_div(mck_rate, iso7816conf->clk);
+ cd = mck_rate;
+ return cd;
+}
+
+static unsigned int atmel_calc_fidi(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ u64 fidi = 0;
+
+ if (iso7816conf->sc_fi && iso7816conf->sc_di) {
+ fidi = (u64)iso7816conf->sc_fi;
+ do_div(fidi, iso7816conf->sc_di);
+ }
+ return (u32)fidi;
+}
+
+/* Enable or disable the iso7816 support */
+/* Called with interrupts disabled */
+static int atmel_config_iso7816(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int mode;
+ unsigned int cd, fidi;
+ int ret = 0;
+
+ /* Disable interrupts */
+ atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
+
+ mode = atmel_uart_readl(port, ATMEL_US_MR);
+
+ if (iso7816conf->flags & SER_ISO7816_ENABLED) {
+ mode &= ~ATMEL_US_USMODE;
+
+ if (iso7816conf->tg > 255) {
+ dev_err(port->dev, "ISO7816: Timeguard exceeding
255\n");
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(0)) {
+ mode |= ATMEL_US_USMODE_ISO7816_T0 | ATMEL_US_DSNACK;
+ } else if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(1)) {
+ mode |= ATMEL_US_USMODE_ISO7816_T1 | ATMEL_US_INACK;
+ } else {
+ dev_err(port->dev, "ISO7816: Type not supported\n");
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ mode &= ~(ATMEL_US_USCLKS | ATMEL_US_NBSTOP | ATMEL_US_PAR);
+
+ /* select mck clock, and output */
+ mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
+ /* set parity for normal/inverse mode + max iterations */
+ mode |= ATMEL_US_PAR_EVEN | ATMEL_US_NBSTOP_1 |
ATMEL_US_MAX_ITER(3);
+
+ cd = atmel_calc_cd(port, iso7816conf);
+ fidi = atmel_calc_fidi(port, iso7816conf);
+ if (fidi == 0) {
+ dev_warn(port->dev, "ISO7816 fidi = 0, Generator
generates no signal\n");
+ } else if (fidi < atmel_port->fidi_min
+ || fidi > atmel_port->fidi_max) {
+ dev_err(port->dev, "ISO7816 fidi = %u, value not
supp
[PATCH v5 1/2] tty/serial_core: add ISO7816 infrastructure
From: Nicolas Ferre
Add the ISO7816 ioctl and associated accessors and data structure.
Drivers can then use this common implementation to handle ISO7816
(smart cards).
Signed-off-by: Nicolas Ferre
[ludovic.desroc...@microchip.com: squash and rebase, removal of gpios,
checkpatch fixes]
Signed-off-by: Ludovic Desroches
---
Documentation/serial/serial-iso7816.txt | 83 +
arch/alpha/include/uapi/asm/ioctls.h| 2 +
arch/mips/include/uapi/asm/ioctls.h | 2 +
arch/parisc/include/uapi/asm/ioctls.h | 2 +
arch/powerpc/include/uapi/asm/ioctls.h | 2 +
arch/sh/include/uapi/asm/ioctls.h | 2 +
arch/sparc/include/uapi/asm/ioctls.h| 2 +
arch/xtensa/include/uapi/asm/ioctls.h | 2 +
drivers/tty/serial/serial_core.c| 60
include/linux/serial_core.h | 3 ++
include/uapi/asm-generic/ioctls.h | 2 +
include/uapi/linux/serial.h | 17 +++
12 files changed, 179 insertions(+)
create mode 100644 Documentation/serial/serial-iso7816.txt
diff --git a/Documentation/serial/serial-iso7816.txt
b/Documentation/serial/serial-iso7816.txt
new file mode 100644
index ..3193d24a2b0f
--- /dev/null
+++ b/Documentation/serial/serial-iso7816.txt
@@ -0,0 +1,83 @@
+ISO7816 SERIAL COMMUNICATIONS
+
+1. INTRODUCTION
+
+ ISO/IEC7816 is a series of standards specifying integrated circuit cards
(ICC)
+ also known as smart cards.
+
+2. HARDWARE-RELATED CONSIDERATIONS
+
+ Some CPUs/UARTs (e.g., Microchip AT91) contain a built-in mode capable of
+ handling communication with a smart card.
+
+ For these microcontrollers, the Linux driver should be made capable of
+ working in both modes, and proper ioctls (see later) should be made
+ available at user-level to allow switching from one mode to the other, and
+ vice versa.
+
+3. DATA STRUCTURES ALREADY AVAILABLE IN THE KERNEL
+
+ The Linux kernel provides the serial_iso7816 structure (see [1]) to handle
+ ISO7816 communications. This data structure is used to set and configure
+ ISO7816 parameters in ioctls.
+
+ Any driver for devices capable of working both as RS232 and ISO7816 should
+ implement the iso7816_config callback in the uart_port structure. The
+ serial_core calls iso7816_config to do the device specific part in response
+ to TIOCGISO7816 and TIOCSISO7816 ioctls (see below). The iso7816_config
+ callback receives a pointer to struct serial_iso7816.
+
+4. USAGE FROM USER-LEVEL
+
+ From user-level, ISO7816 configuration can be get/set using the previous
+ ioctls. For instance, to set ISO7816 you can use the following code:
+
+ #include
+
+ /* Include definition for ISO7816 ioctls: TIOCSISO7816 and TIOCGISO7816
*/
+ #include
+
+ /* Open your specific device (e.g., /dev/mydevice): */
+ int fd = open ("/dev/mydevice", O_RDWR);
+ if (fd < 0) {
+ /* Error handling. See errno. */
+ }
+
+ struct serial_iso7816 iso7816conf;
+
+ /* Reserved fields as to be zeroed */
+ memset(, 0, sizeof(iso7816conf));
+
+ /* Enable ISO7816 mode: */
+ iso7816conf.flags |= SER_ISO7816_ENABLED;
+
+ /* Select the protocol: */
+ /* T=0 */
+ iso7816conf.flags |= SER_ISO7816_T(0);
+ /* or T=1 */
+ iso7816conf.flags |= SER_ISO7816_T(1);
+
+ /* Set the guard time: */
+ iso7816conf.tg = 2;
+
+ /* Set the clock frequency*/
+ iso7816conf.clk = 3571200;
+
+ /* Set transmission factors: */
+ iso7816conf.sc_fi = 372;
+ iso7816conf.sc_di = 1;
+
+ if (ioctl(fd_usart, TIOCSISO7816, ) < 0) {
+ /* Error handling. See errno. */
+ }
+
+ /* Use read() and write() syscalls here... */
+
+ /* Close the device when finished: */
+ if (close (fd) < 0) {
+ /* Error handling. See errno. */
+ }
+
+5. REFERENCES
+
+ [1]include/uapi/linux/serial.h
diff --git a/arch/alpha/include/uapi/asm/ioctls.h
b/arch/alpha/include/uapi/asm/ioctls.h
index 3729d92d3fa8..1e9121c9b3c7 100644
--- a/arch/alpha/include/uapi/asm/ioctls.h
+++ b/arch/alpha/include/uapi/asm/ioctls.h
@@ -102,6 +102,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER_IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define TIOCSERCONFIG 0x5453
#define TIOCSERGWILD 0x5454
diff --git a/arch/mips/include/uapi/asm/ioctls.h
b/arch/mips/include/uapi/asm/ioctls.h
index 890245a9f0c4..16aa8a766aec 100644
--- a/arch/mips/include/uapi/asm/ioctls.h
+++ b/arch/mips/include/uapi/asm/ioctls.h
@@ -93,6 +93,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get e
