* [PATCH 1/2] Documentation: omap-usb-host: fix OMAP OHCI/EHCI file names
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2016-12-15 15:56 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, Yegor Yefremov
From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
OMAP related files are actually named ehci-omap.txt and ohci-omap3.txt.
Also add full path to ohci-omap3.txt.
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
index 4721b2d..aa1eaa5 100644
--- a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
+++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
@@ -64,8 +64,8 @@ Required properties if child node exists:
Properties for children:
The OMAP HS USB Host subsystem contains EHCI and OHCI controllers.
-See Documentation/devicetree/bindings/usb/omap-ehci.txt and
-omap3-ohci.txt
+See Documentation/devicetree/bindings/usb/ehci-omap.txt and
+Documentation/devicetree/bindings/usb/ohci-omap3.txt.
Example for OMAP4:
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2] i2c: designware: Cleaning and comment style fixes.
From: Luis Oliveira @ 2016-12-15 15:53 UTC (permalink / raw)
To: Peter Rosin, Luis Oliveira, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <f11794ec-439b-5fcd-65f4-c14c319cd627-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 15-Dec-16 15:17, Peter Rosin wrote:
> On 2016-12-15 15:38, Luis Oliveira wrote:
>> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
>> index 26250b425e2f..3cb81fca7738 100644
>> --- a/drivers/i2c/busses/i2c-designware-core.h
>> +++ b/drivers/i2c/busses/i2c-designware-core.h
>> @@ -36,7 +36,7 @@
>> #define DW_IC_CON_SPEED_FAST 0x4
>> #define DW_IC_CON_SPEED_HIGH 0x6
>> #define DW_IC_CON_SPEED_MASK 0x6
>> -#define DW_IC_CON_10BITADDR_MASTER 0x10
>> +#define DW_IC_CON_10BITADDR_MASTER 0x10
>
> How is this an improvement?
It is not, I will fix it.
Thanks,
Luis
>
> Cheers,
> peda
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 2/6] [media] rc-main: split setup and unregister functions
From: Sean Young @ 2016-12-15 15:50 UTC (permalink / raw)
To: Andi Shyti
Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Richard Purdie,
Jacek Anaszewski, Heiner Kallweit, linux-media, devicetree,
linux-leds, linux-kernel, Andi Shyti
In-Reply-To: <20161214140030.28537-3-andi.shyti@samsung.com>
Hi Andi,
This patch breaks all rc devices, none of them have input devices any
more (see below).
On Wed, Dec 14, 2016 at 11:00:26PM +0900, Andi Shyti wrote:
> Move the input device allocation, map and protocol handling to
> different functions.
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> Reviewed-by: Sean Young <sean@mess.org>
> ---
> drivers/media/rc/rc-main.c | 143 +++++++++++++++++++++++++--------------------
> 1 file changed, 81 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index a6bbceb..59fac96 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1436,16 +1436,12 @@ struct rc_dev *devm_rc_allocate_device(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(devm_rc_allocate_device);
>
> -int rc_register_device(struct rc_dev *dev)
> +static int rc_setup_rx_device(struct rc_dev *dev)
> {
> - static bool raw_init = false; /* raw decoders loaded? */
> - struct rc_map *rc_map;
> - const char *path;
> - int attr = 0;
> - int minor;
> int rc;
> + struct rc_map *rc_map;
>
> - if (!dev || !dev->map_name)
> + if (!dev->map_name)
> return -EINVAL;
>
> rc_map = rc_map_get(dev->map_name);
> @@ -1454,6 +1450,19 @@ int rc_register_device(struct rc_dev *dev)
> if (!rc_map || !rc_map->scan || rc_map->size == 0)
> return -EINVAL;
>
> + rc = ir_setkeytable(dev, rc_map);
> + if (rc)
> + return rc;
> +
> + if (dev->change_protocol) {
> + u64 rc_type = (1ll << rc_map->rc_type);
> +
> + rc = dev->change_protocol(dev, &rc_type);
> + if (rc < 0)
> + goto out_table;
> + dev->enabled_protocols = rc_type;
> + }
> +
> set_bit(EV_KEY, dev->input_dev->evbit);
> set_bit(EV_REP, dev->input_dev->evbit);
> set_bit(EV_MSC, dev->input_dev->evbit);
> @@ -1463,6 +1472,61 @@ int rc_register_device(struct rc_dev *dev)
> if (dev->close)
> dev->input_dev->close = ir_close;
>
> + /*
> + * Default delay of 250ms is too short for some protocols, especially
> + * since the timeout is currently set to 250ms. Increase it to 500ms,
> + * to avoid wrong repetition of the keycodes. Note that this must be
> + * set after the call to input_register_device().
> + */
> + dev->input_dev->rep[REP_DELAY] = 500;
> +
> + /*
> + * As a repeat event on protocols like RC-5 and NEC take as long as
> + * 110/114ms, using 33ms as a repeat period is not the right thing
> + * to do.
> + */
> + dev->input_dev->rep[REP_PERIOD] = 125;
> +
> + /* rc_open will be called here */
> + rc = input_register_device(dev->input_dev);
> + if (rc)
> + goto out_table;
> +
> + dev->input_dev->dev.parent = &dev->dev;
> + memcpy(&dev->input_dev->id, &dev->input_id, sizeof(dev->input_id));
> + dev->input_dev->phys = dev->input_phys;
> + dev->input_dev->name = dev->input_name;
I was testing your changes, and with this patch none of my rc devices
have input devices associated with them. The problem is that you've changed
the order: input_register_device() should happen AFTER the preceding
4 lines.
> +
> + return 0;
> +
> +out_table:
> + ir_free_table(&dev->rc_map);
> +
> + return rc;
> +}
> +
> +static void rc_free_rx_device(struct rc_dev *dev)
> +{
> + if (!dev)
> + return;
> +
> + ir_free_table(&dev->rc_map);
> +
> + input_unregister_device(dev->input_dev);
> + dev->input_dev = NULL;
> +}
> +
> +int rc_register_device(struct rc_dev *dev)
> +{
> + static bool raw_init = false; /* raw decoders loaded? */
> + const char *path;
> + int attr = 0;
> + int minor;
> + int rc;
> +
> + if (!dev)
> + return -EINVAL;
> +
> minor = ida_simple_get(&rc_ida, 0, RC_DEV_MAX, GFP_KERNEL);
> if (minor < 0)
> return minor;
> @@ -1486,39 +1550,15 @@ int rc_register_device(struct rc_dev *dev)
> if (rc)
> goto out_unlock;
>
> - rc = ir_setkeytable(dev, rc_map);
> - if (rc)
> - goto out_dev;
See the original order here.
> -
> - dev->input_dev->dev.parent = &dev->dev;
> - memcpy(&dev->input_dev->id, &dev->input_id, sizeof(dev->input_id));
> - dev->input_dev->phys = dev->input_phys;
> - dev->input_dev->name = dev->input_name;
> -
> - rc = input_register_device(dev->input_dev);
> - if (rc)
> - goto out_table;
> -
> - /*
> - * Default delay of 250ms is too short for some protocols, especially
> - * since the timeout is currently set to 250ms. Increase it to 500ms,
> - * to avoid wrong repetition of the keycodes. Note that this must be
> - * set after the call to input_register_device().
> - */
> - dev->input_dev->rep[REP_DELAY] = 500;
> -
> - /*
> - * As a repeat event on protocols like RC-5 and NEC take as long as
> - * 110/114ms, using 33ms as a repeat period is not the right thing
> - * to do.
> - */
> - dev->input_dev->rep[REP_PERIOD] = 125;
> -
> path = kobject_get_path(&dev->dev.kobj, GFP_KERNEL);
> dev_info(&dev->dev, "%s as %s\n",
> dev->input_name ?: "Unspecified device", path ?: "N/A");
> kfree(path);
>
> + rc = rc_setup_rx_device(dev);
> + if (rc)
> + goto out_dev;
> +
> if (dev->driver_type == RC_DRIVER_IR_RAW) {
> if (!raw_init) {
> request_module_nowait("ir-lirc-codec");
> @@ -1526,36 +1566,20 @@ int rc_register_device(struct rc_dev *dev)
> }
> rc = ir_raw_event_register(dev);
> if (rc < 0)
> - goto out_input;
> - }
> -
> - if (dev->change_protocol) {
> - u64 rc_type = (1ll << rc_map->rc_type);
> - rc = dev->change_protocol(dev, &rc_type);
> - if (rc < 0)
> - goto out_raw;
> - dev->enabled_protocols = rc_type;
> + goto out_rx;
> }
>
> /* Allow the RC sysfs nodes to be accessible */
> atomic_set(&dev->initialized, 1);
>
> - IR_dprintk(1, "Registered rc%u (driver: %s, remote: %s, mode %s)\n",
> + IR_dprintk(1, "Registered rc%u (driver: %s)\n",
> dev->minor,
> - dev->driver_name ? dev->driver_name : "unknown",
> - rc_map->name ? rc_map->name : "unknown",
> - dev->driver_type == RC_DRIVER_IR_RAW ? "raw" : "cooked");
> + dev->driver_name ? dev->driver_name : "unknown");
>
> return 0;
>
> -out_raw:
> - if (dev->driver_type == RC_DRIVER_IR_RAW)
> - ir_raw_event_unregister(dev);
> -out_input:
> - input_unregister_device(dev->input_dev);
> - dev->input_dev = NULL;
> -out_table:
> - ir_free_table(&dev->rc_map);
> +out_rx:
> + rc_free_rx_device(dev);
> out_dev:
> device_del(&dev->dev);
> out_unlock:
> @@ -1601,12 +1625,7 @@ void rc_unregister_device(struct rc_dev *dev)
> if (dev->driver_type == RC_DRIVER_IR_RAW)
> ir_raw_event_unregister(dev);
>
> - /* Freeing the table should also call the stop callback */
> - ir_free_table(&dev->rc_map);
> - IR_dprintk(1, "Freed keycode table\n");
> -
> - input_unregister_device(dev->input_dev);
> - dev->input_dev = NULL;
> + rc_free_rx_device(dev);
>
> device_del(&dev->dev);
>
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2016-12-15 15:26 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: devicetree, Alexandre Torgue, Wolfram Sang, linux-kernel,
Linus Walleij, Patrice Chotard, linux, Rob Herring, linux-i2c,
Maxime Coquelin, linux-arm-kernel
In-Reply-To: <20161213092031.d2ax2pnpzzicriel@pengutronix.de>
Hello,
Thanks for this review, it will help.
2016-12-13 10:20 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> Hello,
>
> On Mon, Dec 12, 2016 at 05:15:39PM +0100, M'boumba Cedric Madianga wrote:
>> This patch adds support for the STM32F4 I2C controller.
>>
>> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> ---
>> drivers/i2c/busses/Kconfig | 10 +
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-stm32f4.c | 849 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 860 insertions(+)
>> create mode 100644 drivers/i2c/busses/i2c-stm32f4.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 0cdc844..2719208 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -886,6 +886,16 @@ config I2C_ST
>> This driver can also be built as module. If so, the module
>> will be called i2c-st.
>>
>> +config I2C_STM32F4
>> + tristate "STMicroelectronics STM32F4 I2C support"
>> + depends on ARCH_STM32 || COMPILE_TEST
>> + help
>> + Enable this option to add support for STM32 I2C controller embedded
>> + in STM32F4 SoCs.
>> +
>> + This driver can also be built as module. If so, the module
>> + will be called i2c-stm32f4.
>> +
>> config I2C_STU300
>> tristate "ST Microelectronics DDC I2C interface"
>> depends on MACH_U300
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index 1c1bac8..a2c6ff5 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -85,6 +85,7 @@ obj-$(CONFIG_I2C_SH_MOBILE) += i2c-sh_mobile.o
>> obj-$(CONFIG_I2C_SIMTEC) += i2c-simtec.o
>> obj-$(CONFIG_I2C_SIRF) += i2c-sirf.o
>> obj-$(CONFIG_I2C_ST) += i2c-st.o
>> +obj-$(CONFIG_I2C_STM32F4) += i2c-stm32f4.o
>> obj-$(CONFIG_I2C_STU300) += i2c-stu300.o
>> obj-$(CONFIG_I2C_SUN6I_P2WI) += i2c-sun6i-p2wi.o
>> obj-$(CONFIG_I2C_TEGRA) += i2c-tegra.o
>> diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
>> new file mode 100644
>> index 0000000..89ad579
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-stm32f4.c
>> @@ -0,0 +1,849 @@
>> +/*
>> + * Driver for STMicroelectronics STM32 I2C controller
>> + *
>> + * Copyright (C) M'boumba Cedric Madianga 2015
>> + * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> + *
>> + * This driver is based on i2c-st.c
>> + *
>> + * License terms: GNU General Public License (GPL), version 2
>> + */
>
> If there is a public description available for the device, a link here
> would be great.
The device is described in the reference manual of the STM32F429/439 SoC.
As this reference manual is public, I will add it at the beginning of
the driver as requested.
>
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +
>> +/* STM32F4 I2C offset registers */
>> +#define STM32F4_I2C_CR1 0x00
>> +#define STM32F4_I2C_CR2 0x04
>> +#define STM32F4_I2C_DR 0x10
>> +#define STM32F4_I2C_SR1 0x14
>> +#define STM32F4_I2C_SR2 0x18
>> +#define STM32F4_I2C_CCR 0x1C
>> +#define STM32F4_I2C_TRISE 0x20
>> +#define STM32F4_I2C_FLTR 0x24
>> +
>> +/* STM32F4 I2C control 1*/
>> +#define STM32F4_I2C_CR1_SWRST BIT(15)
>> +#define STM32F4_I2C_CR1_POS BIT(11)
>> +#define STM32F4_I2C_CR1_ACK BIT(10)
>> +#define STM32F4_I2C_CR1_STOP BIT(9)
>> +#define STM32F4_I2C_CR1_START BIT(8)
>> +#define STM32F4_I2C_CR1_PE BIT(0)
>> +
>> +/* STM32F4 I2C control 2 */
>> +#define STM32F4_I2C_CR2_FREQ_MASK GENMASK(5, 0)
>> +#define STM32F4_I2C_CR2_FREQ(n) ((n & STM32F4_I2C_CR2_FREQ_MASK))
>
> This should better be ((n) & STM32F4_I2C_CR2_FREQ_MASK). There a few
> more constants that need the same fix.
OK I will fix it in the V7. Thanks.
>
>> +#define STM32F4_I2C_CR2_ITBUFEN BIT(10)
>> +#define STM32F4_I2C_CR2_ITEVTEN BIT(9)
>> +#define STM32F4_I2C_CR2_ITERREN BIT(8)
>> +#define STM32F4_I2C_CR2_IRQ_MASK (STM32F4_I2C_CR2_ITBUFEN \
>> + | STM32F4_I2C_CR2_ITEVTEN \
>> + | STM32F4_I2C_CR2_ITERREN)
>
> I'd layout this like:
>
> #define STM32F4_I2C_CR2_IRQ_MASK (STM32F4_I2C_CR2_ITBUFEN | \
> STM32F4_I2C_CR2_ITEVTEN | \
> STM32F4_I2C_CR2_ITERREN)
>
> which is more usual I think.
OK I will fix it in the V7. Thanks.
>
>> +/* STM32F4 I2C Status 1 */
>> +#define STM32F4_I2C_SR1_AF BIT(10)
>> +#define STM32F4_I2C_SR1_ARLO BIT(9)
>> +#define STM32F4_I2C_SR1_BERR BIT(8)
>> +#define STM32F4_I2C_SR1_TXE BIT(7)
>> +#define STM32F4_I2C_SR1_RXNE BIT(6)
>> +#define STM32F4_I2C_SR1_BTF BIT(2)
>> +#define STM32F4_I2C_SR1_ADDR BIT(1)
>> +#define STM32F4_I2C_SR1_SB BIT(0)
>> +#define STM32F4_I2C_SR1_ITEVTEN_MASK (STM32F4_I2C_SR1_BTF \
>> + | STM32F4_I2C_SR1_ADDR \
>> + | STM32F4_I2C_SR1_SB)
>> +#define STM32F4_I2C_SR1_ITBUFEN_MASK (STM32F4_I2C_SR1_TXE \
>> + | STM32F4_I2C_SR1_RXNE)
>> +#define STM32F4_I2C_SR1_ITERREN_MASK (STM32F4_I2C_SR1_AF \
>> + | STM32F4_I2C_SR1_ARLO \
>> + | STM32F4_I2C_SR1_BERR)
>> +
>> +/* STM32F4 I2C Status 2 */
>> +#define STM32F4_I2C_SR2_BUSY BIT(1)
>> +
>> +/* STM32F4 I2C Control Clock */
>> +#define STM32F4_I2C_CCR_CCR_MASK GENMASK(11, 0)
>> +#define STM32F4_I2C_CCR_CCR(n) ((n & STM32F4_I2C_CCR_CCR_MASK))
>> +#define STM32F4_I2C_CCR_FS BIT(15)
>> +#define STM32F4_I2C_CCR_DUTY BIT(14)
>> +
>> +/* STM32F4 I2C Trise */
>> +#define STM32F4_I2C_TRISE_VALUE_MASK GENMASK(5, 0)
>> +#define STM32F4_I2C_TRISE_VALUE(n) ((n & STM32F4_I2C_TRISE_VALUE_MASK))
>> +
>> +/* STM32F4 I2C Filter */
>> +#define STM32F4_I2C_FLTR_DNF_MASK GENMASK(3, 0)
>> +#define STM32F4_I2C_FLTR_DNF(n) ((n & STM32F4_I2C_FLTR_DNF_MASK))
>> +#define STM32F4_I2C_FLTR_ANOFF BIT(4)
>> +
>> +#define STM32F4_I2C_MIN_FREQ 2U
>> +#define STM32F4_I2C_MAX_FREQ 42U
>> +#define FAST_MODE_MAX_RISE_TIME 1000
>> +#define STD_MODE_MAX_RISE_TIME 300
>
> Are these supposed to be the values "rise time of both SDA and SCL
> signals" from the i2c specification? If so, you got it wrong, fast mode
> has the smaller value.
Yes you are right. My mistake. Thanks
> Maybe these constants could get a home in a more central place?
Yes we probably could add these constants in a include file like i2c.h
or someting like that.
Wolfram, what is your opinion regarding this proposal ?
> Also I'd add /* ns */ to the definition.
Ok
>
>> +#define MHZ_TO_HZ 1000000
>> +
>> +enum stm32f4_i2c_speed {
>> + STM32F4_I2C_SPEED_STANDARD, /* 100 kHz */
>> + STM32F4_I2C_SPEED_FAST, /* 400 kHz */
>> + STM32F4_I2C_SPEED_END,
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_timings - per-Mode tuning parameters
>> + * @duty: Fast mode duty cycle
>> + * @mul_ccr: Value to be multiplied to CCR to reach 100Khz/400Khz SCL frequency
>> + * @min_ccr: Minimum clock ctrl reg value to reach 100Khz/400Khz SCL frequency
>> + */
>> +struct stm32f4_i2c_timings {
>> + u32 rate;
>
> rate is undocumented and unused.
Good point. I will remove it. Thanks.
>
>> + u32 duty;
>> + u32 mul_ccr;
>> + u32 min_ccr;
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_msg - client specific data
>> + * @addr: 8-bit slave addr, including r/w bit
>> + * @count: number of bytes to be transferred
>> + * @buf: data buffer
>> + * @result: result of the transfer
>> + * @stop: last I2C msg to be sent, i.e. STOP to be generated
>> + */
>> +struct stm32f4_i2c_msg {
>> + u8 addr;
>> + u32 count;
>> + u8 *buf;
>> + int result;
>> + bool stop;
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_dev - private data of the controller
>> + * @adap: I2C adapter for this controller
>> + * @dev: device for this controller
>> + * @base: virtual memory area
>> + * @complete: completion of I2C message
>> + * @irq_event: interrupt event line for the controller
>> + * @irq_error: interrupt error line for the controller
>> + * @clk: hw i2c clock
>> + * speed: I2C clock frequency of the controller. Standard or Fast only supported
>> + * @msg: I2C transfer information
>> + */
>> +struct stm32f4_i2c_dev {
>> + struct i2c_adapter adap;
>> + struct device *dev;
>> + void __iomem *base;
>> + struct completion complete;
>> + int irq_event;
>> + int irq_error;
>
> You only use irq_event in the probe function. So there is no need to
> remember this one and you could use a local variable instead.
Ok, I will fix it in the V7. Thanks
>
>> + struct clk *clk;
>> + int speed;
>> + struct stm32f4_i2c_msg msg;
>> +};
>> +
>> +static struct stm32f4_i2c_timings i2c_timings[] = {
>> + [STM32F4_I2C_SPEED_STANDARD] = {
>> + .mul_ccr = 1,
>> + .min_ccr = 4,
>> + .duty = 0,
>> + },
>> + [STM32F4_I2C_SPEED_FAST] = {
>> + .mul_ccr = 16,
>> + .min_ccr = 1,
>> + .duty = 1,
>> + },
>
> Are these values from the datasheet?
Yes, these values come from I2C IP datasheet.
>
>> +};
>> +
>> +static inline void stm32f4_i2c_set_bits(void __iomem *reg, u32 mask)
>> +{
>> + writel_relaxed(readl_relaxed(reg) | mask, reg);
>> +}
>> +
>> +static inline void stm32f4_i2c_clr_bits(void __iomem *reg, u32 mask)
>> +{
>> + writel_relaxed(readl_relaxed(reg) & ~mask, reg);
>> +}
>> +
>> +static void stm32f4_i2c_soft_reset(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_SWRST);
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_SWRST);
>
> Not very critical, but you're doing an unneeded register access here
> because the register is read twice.
>
> Also I think readability would improve if you dropped
> stm32f4_i2c_{set,clr}_bits and do their logic explicitly in the callers.
>
> stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_SWRST);
> stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_SWRST);
>
> vs
>
> val = readl_relaxed(reg);
> writel_relaxed(val | STM32F4_I2C_CR1_SWRST, reg);
> writel_relaxed(val, reg);
>
If it is just for this function, I don't have any objection to use
your proposal.
But if your request, it is to drop all calls of
stm32f4_i2c_{set,clr}_bits, I am wondering if it is something really
critical ?
Indeed, this is a big impact in this driver and I would prefer to avoid it.
>> +}
>> +
>> +static void stm32f4_i2c_disable_it(struct stm32f4_i2c_dev *i2c_dev)
>
> What is "it"? If it stands for "interrupt" the more usual abbrev is
> "irq".
Yes it stands for interrupt.
So, I will replace it by irq in the next version.
>
>> +{
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_IRQ_MASK);
>> +}
>> +
>> +static void stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + u32 clk_rate, cr2, freq;
>> +
>> + cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> + cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
>> + clk_rate = clk_get_rate(i2c_dev->clk);
>> + freq = clk_rate / MHZ_TO_HZ;
>> + freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
>> + cr2 |= STM32F4_I2C_CR2_FREQ(freq);
>> + writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
>
> Can you quote the data sheet enough in a comment here to make it obvious
> that your calculation is right?
Ok I will add it.
>
> Would it be more sensible to error out if clk_rate / MHZ_TO_HZ isn't in
> the interval [STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ]?
Yes, input clock must be between 2 MHz and 42 Mhz to achieve standard
or fast mode mode I²C frequencies.
If it is not the case, the I2C signal integrity is not guarantee and
could lead to communication issue between devices on the I2C bus.
>
> Usually I would expect that you need to use
> DIV_ROUND_UP(clk_rate, MHZ_TO_HZ) instead of a plain division.
Ok, I will use this macro in the next version
>
>> +}
>> +
>> +static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + u32 trise, freq, cr2, val;
>> +
>> + cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> + freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
>> +
>> + trise = readl_relaxed(i2c_dev->base + STM32F4_I2C_TRISE);
>> + trise &= ~STM32F4_I2C_TRISE_VALUE_MASK;
>
> Are you required to use rmw for STM32F4_I2C_TRISE? I'd prefer
>
> writel_relaxed(STM32F4_I2C_TRISE_VALUE(..), i2c_dev->base + STM32F4_I2C_TRISE);
>
> unless the datasheet requires rmw.
>
>> + /* Maximum rise time computation */
>> + if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD) {
>> + trise |= STM32F4_I2C_TRISE_VALUE((freq + 1));
>
> A single pair of parenthesis is enough when you fix
> STM32F4_I2C_TRISE_VALUE as suggested above.
The datasheet does not required to use rmw so I will use your proposal
in the next version. Thanks.
>
>> + } else {
>> + val = freq * FAST_MODE_MAX_RISE_TIME / STD_MODE_MAX_RISE_TIME;
>> + trise |= STM32F4_I2C_TRISE_VALUE((val + 1));
>
> val could be local to this branch.
>
> Or make it shorter using:
>
> freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
> if (i2c_dev->speed == STM32F4_I2C_SPEED_FAST)
> freq = freq * FAST_MODE_MAX_RISE_TIME / STD_MODE_MAX_RISE_TIME;
>
> writel_relaxed(STM32F4_I2C_TRISE_VALUE(freq + 1), ...);
>
> A quote from the data sheet about the algorithm would be good here, too.
Ok, I will use a shorter way to compute freq and add a quote from the datasheet
>
>> + }
>> +
>> + writel_relaxed(trise, i2c_dev->base + STM32F4_I2C_TRISE);
>> +}
>> +
>> +static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_timings *t = &i2c_timings[i2c_dev->speed];
>> + u32 ccr, clk_rate;
>> + int val;
>> +
>> + ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
>> + ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
>> + STM32F4_I2C_CCR_CCR_MASK);
>> +
>> + clk_rate = clk_get_rate(i2c_dev->clk);
>> + val = clk_rate / MHZ_TO_HZ * t->mul_ccr;
>
> Is the rounding done right? Again please describe the hardware in a
> comment.
Ok I will use DIV_ROUND_UP here and add a comment from datasheet
>
>> + if (val < t->min_ccr)
>> + val = t->min_ccr;
>> + ccr |= STM32F4_I2C_CCR_CCR(val);
>> +
>> + if (t->duty)
>> + ccr |= STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY;
>> +
>> + writel_relaxed(ccr, i2c_dev->base + STM32F4_I2C_CCR);
>> +}
>> +[...]
>> +
>> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + u32 status;
>> + int ret;
>> +
>> + ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
>> + status,
>> + !(status & STM32F4_I2C_SR2_BUSY),
>> + 10, 1000);
>> + if (ret) {
>> + dev_err(i2c_dev->dev, "bus not free\n");
>> + ret = -EBUSY;
>
> I'm not sure if "bus not free" deserves an error message. Wolfram?
>
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +[...]
>> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + u32 rbuf;
>> +
>> + rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
>> + *msg->buf++ = (u8)rbuf & 0xff;
>
> unneeded cast (or unneeded & 0xff).
Ok thanks
>
>> + msg->count--;
>> +}
>> +
>> +[...]
>> +/**
>> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> + switch (msg->count) {
>> + case 1:
>> + stm32f4_i2c_disable_it(i2c_dev);
>> + stm32f4_i2c_read_msg(i2c_dev);
>> + complete(&i2c_dev->complete);
>> + break;
>> + case 2:
>> + case 3:
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> + break;
>> + default:
>> + stm32f4_i2c_read_msg(i2c_dev);
>> + }
>
> It looks wrong that you don't call stm32f4_i2c_read_msg if msg->count is
> 2 or 3. I guess that's because these cases are handled in
> stm32f4_i2c_handle_rx_btf? Maybe you can simplify the logic a bit?
stm32f4_i2c_handle_read is called when RXNE bit is set due to new data
present in DR register.
stm32f4_i2c_handle_rx_btf is called when BTF bit is set.
This bit is set when there is new data in shift register whereas data
in DR register has not been read yet.
The datasheet requires to wait for BTF bit for 2 byte reception and
for the 3 last bytes to be read for N bytes reception (with N > 2)
That's why, in these cases (2 and 3), I clear the ITBUF interrupt in
order to not be preempted again for RXNE event as I know that I have
to wait for BTF event.
So, this is not a wrong case but I will add a comment to explain that.
>
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
>> + * in case of read
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + void __iomem *reg;
>> + u32 mask;
>> + int i;
>> +
>> + switch (msg->count) {
>
> I don't understand why the handling depends on the number of messages.
Please see my above comment
>
>> + case 2:
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + /* Generate STOP or REPSTART */
>
> I stumbled about "REPSTART" and would spell it out as "repeated Start".
Ok
>
>> + if (msg->stop)
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> + else
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +
>> + /* Read two last data bytes */
>> + for (i = 2; i > 0; i--)
>> + stm32f4_i2c_read_msg(i2c_dev);
>> +
>> + /* Disable EVT and ERR interrupt */
>> + reg = i2c_dev->base + STM32F4_I2C_CR2;
>> + mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
>> + stm32f4_i2c_clr_bits(reg, mask);
>> +
>> + complete(&i2c_dev->complete);
>> + break;
>> + case 3:
>> + /* Enable ACK and read data */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> + stm32f4_i2c_read_msg(i2c_dev);
>> + break;
>> + default:
>> + stm32f4_i2c_read_msg(i2c_dev);
>> + }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
>> + * master receiver
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + void __iomem *reg;
>> +
>> + switch (msg->count) {
>> + case 0:
>> + stm32f4_i2c_terminate_xfer(i2c_dev);
>> + /* Clear ADDR flag */
>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> + break;
>> + case 1:
>> + /*
>> + * Single byte reception:
>> + * Enable NACK, clear ADDR flag and generate STOP or RepSTART
>> + */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> + if (msg->stop)
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> + else
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> + break;
>> + case 2:
>> + /*
>> + * 2-byte reception:
>> + * Enable NACK and PEC Position Ack and clear ADDR flag
>
> What is PEC?
PEC stands for Packet Error Checking.
This feature is used is SMbus mode in order to guarantee data
integrity during an I2C transaction thanks to packet error code
comparaison.
The POS bit is used in reception to indicate that the next byte
received in the shift register will be ACK by hardware.
So, this is a wrong comment I would say POS ACK and I will fix it in
the next version.
>
>> + */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> + break;
>> +
>> + default:
>> + /* N-byte reception: Enable ACK and clear ADDR flag */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> + break;
>> + }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_isr_event() - Interrupt routine for I2C bus event
>> + * @irq: interrupt number
>> + * @data: Controller's private data
>> + */
>> +static irqreturn_t stm32f4_i2c_isr_event(int irq, void *data)
>> +{
>> +[...]
>> + real_status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
>
> s/real_status/status/ ?
Ok. Thanks
>
>> +
>> + if (!(real_status & possible_status)) {
>> + dev_dbg(i2c_dev->dev,
>> + "spurious evt it (status=0x%08x, ien=0x%08x)\n",
>> + real_status, ien);
>
> s/it/irq/
Ok. Thanks
>
>> + return IRQ_NONE;
>> + }
>> +
>> + /* Use __fls() to check error bits first */
>> + flag = __fls(real_status & possible_status);
>
> If you get several events reported you only handle a single one. Is this
> effective?
You are right, if several events occur, I will execute this irq
routines for each event.
I will rework this in the next version. Thanks
>
>> + switch (1 << flag) {
>> + case STM32F4_I2C_SR1_SB:
>> + stm32f4_i2c_write_byte(i2c_dev, msg->addr);
>> + break;
>> +
>> + case STM32F4_I2C_SR1_ADDR:
>> + if (msg->addr & I2C_M_RD)
>> + stm32f4_i2c_handle_rx_addr(i2c_dev);
>> + else
>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +
>> + /* Enable ITBUF interrupts */
>
> What is ITBUF?
ITBUF is an interrupt generated when RxNE or TxE flag is set
>
>> + reg = i2c_dev->base + STM32F4_I2C_CR2;
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> + break;
>> +
>> + case STM32F4_I2C_SR1_BTF:
>> + if (msg->addr & I2C_M_RD)
>> + stm32f4_i2c_handle_rx_btf(i2c_dev);
>> + else
>> + stm32f4_i2c_handle_write(i2c_dev);
>> + break;
>> +
>> + case STM32F4_I2C_SR1_TXE:
>> + stm32f4_i2c_handle_write(i2c_dev);
>> + break;
>> +
>> + case STM32F4_I2C_SR1_RXNE:
>> + stm32f4_i2c_handle_read(i2c_dev);
>> + break;
>> +
>> + default:
>> + dev_err(i2c_dev->dev,
>> + "evt it unhandled: status=0x%08x)\n", real_status);
>
> s/it/irq/
ok
>
>> + return IRQ_NONE;
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +[...]
>> +static int stm32f4_i2c_xfer_msg(struct stm32f4_i2c_dev *i2c_dev,
>> + struct i2c_msg *msg, bool is_first,
>> + bool is_last)
>> +{
>> +[...]
>> + /* Enable ITEVT and ITERR interrupts */
>
> This comment isn't helpful. Mentioning their meaning would be great
> instead.
ok I will add it (ITEVT = event interrupt and ITERR = error interrupt)
>
>> +[...]
>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[],
>> + int num)
>> +{
>> + struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
>> + int ret, i;
>> +
>> + ret = clk_enable(i2c_dev->clk);
>> + if (ret) {
>> + dev_err(i2c_dev->dev, "Failed to enable clock\n");
>> + return ret;
>> + }
>> +
>> + stm32f4_i2c_hw_config(i2c_dev);
>> +
>> + for (i = 0; i < num && !ret; i++)
>> + ret = stm32f4_i2c_xfer_msg(i2c_dev, &msgs[i], i == 0,
>> + i == num - 1);
>> +
>> + clk_disable(i2c_dev->clk);
>> +
>> + return (ret < 0) ? ret : i;
>
> using num instead of i would be a bit more obvious.
ok
>
>> +static int stm32f4_i2c_probe(struct platform_device *pdev)
>> +{
>> +[...]
>> + i2c_dev->speed = STM32F4_I2C_SPEED_STANDARD;
>> + ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
>> + if ((!ret) && (clk_rate == 400000))
>> + i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
>
> I'd use
>
> if (!ret && clk_rate >= 400000)
> i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
>
> . That's less parenthesis and a more robust selection of the bus
> frequency.
ok
>
>> +
>> + i2c_dev->dev = &pdev->dev;
>> +
>> + ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_event,
>> + NULL, stm32f4_i2c_isr_event,
>> + IRQF_ONESHOT, pdev->name, i2c_dev);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Failed to request irq %i\n",
>> + i2c_dev->irq_error);
>
> That's wrong. Requesting irq_event failed.
Ok Thanks.
>
>> + goto clk_free;
>> + }
>> +
>> + ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_error,
>> + NULL, stm32f4_i2c_isr_error,
>> + IRQF_ONESHOT, pdev->name, i2c_dev);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Failed to request irq %i\n",
>> + i2c_dev->irq_error);
>> + goto clk_free;
>
> It would also be nice to know for which type of irq this failed. I.e.
> please point out if this is the error irq or the event irq in the
> message. Ditto for checking the return type of irq_of_parse_and_map.
Ok I will fix it in the next version.
>
>> + }
>> +
>> + adap = &i2c_dev->adap;
>> + i2c_set_adapdata(adap, i2c_dev);
>> + snprintf(adap->name, sizeof(adap->name), "STM32 I2C(%pa)", &res->start);
>> + adap->owner = THIS_MODULE;
>> + adap->timeout = 2 * HZ;
>> + adap->retries = 0;
>> + adap->algo = &stm32f4_i2c_algo;
>> + adap->dev.parent = &pdev->dev;
>> + adap->dev.of_node = pdev->dev.of_node;
>> +
>> + init_completion(&i2c_dev->complete);
>> +
>> + ret = i2c_add_adapter(adap);
>> + if (ret)
>> + goto clk_free;
>> +
>> + platform_set_drvdata(pdev, i2c_dev);
>> +
>> + dev_info(i2c_dev->dev, "STM32F4 I2C driver initialized\n");
>
> This is wrong. The driver is bound now to a device, not initialized.
Ok I will replaced initialized by registered. Thanks.
>
>> +static const struct of_device_id stm32f4_i2c_match[] = {
>> + { .compatible = "st,stm32f4-i2c", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32f4_i2c_match);
>> +
>> +static struct platform_driver stm32f4_i2c_driver = {
>> + .driver = {
>> + .name = "stm32f4-i2c",
>> + .of_match_table = stm32f4_i2c_match,
>
> Is this needed?
Without of_match_table, I could not match an I2C device instance from
DT with this driver.
So maybe, there is a misunderstanding.
Could you please clarify your question ?
>
>> + },
>> + .probe = stm32f4_i2c_probe,
>> + .remove = stm32f4_i2c_remove,
>> +};
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
Thanks again
Best regards,
Cedric
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] i2c: designware: Cleaning and comment style fixes.
From: Peter Rosin @ 2016-12-15 15:17 UTC (permalink / raw)
To: Luis Oliveira, wsa, robh+dt, mark.rutland, jarkko.nikula,
andriy.shevchenko, mika.westerberg, linux-i2c, devicetree,
linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
In-Reply-To: <d7b5a22ce09539c2af343c21ca69675bef6a0a10.1481812563.git.lolivei@synopsys.com>
On 2016-12-15 15:38, Luis Oliveira wrote:
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index 26250b425e2f..3cb81fca7738 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -36,7 +36,7 @@
> #define DW_IC_CON_SPEED_FAST 0x4
> #define DW_IC_CON_SPEED_HIGH 0x6
> #define DW_IC_CON_SPEED_MASK 0x6
> -#define DW_IC_CON_10BITADDR_MASTER 0x10
> +#define DW_IC_CON_10BITADDR_MASTER 0x10
How is this an improvement?
Cheers,
peda
^ permalink raw reply
* Re: [PATCH 1/1] of: of_reserved_mem: Ensure cma reserved region not cross the low/high memory
From: Jason Liu @ 2016-12-15 15:10 UTC (permalink / raw)
To: Rob Herring
Cc: Laura Abbott, Jason Liu, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Frank Rowand
In-Reply-To: <CAL_JsqJygJK_VmXKad8n63-jLH+G_xQ=NwX8JVmE=xSuYMof=Q@mail.gmail.com>
2016-12-15 21:54 GMT+08:00 Rob Herring <robh+dt@kernel.org>:
> On Wed, Dec 14, 2016 at 4:21 PM, Laura Abbott <labbott@redhat.com> wrote:
>> On 12/14/2016 12:45 PM, Rob Herring wrote:
>>> On Wed, Nov 23, 2016 at 5:37 AM, Jason Liu <jason.hui.liu@nxp.com> wrote:
>>>> Need ensure the cma reserved region not cross the low/high memory boundary
>>>> when using the dynamic allocation methond through device-tree, otherwise,
>>>> kernel will fail to boot up when cma reserved region cross how/high mem.
>>>
>>> The kernel command line code setting CMA already deals with this. Why
>>> don't we just call the CMA code (cma_declare_contiguous) to deal with
>>> this?
>>>
>>> Rob
>>>
>>
>> That was proposed in the first version[1] but I think this is a generic
>> problem not specific to CMA. Even non-CMA reservations trying to span
>> zones could cause problems so the devicetree allocation code should
>> restrict reservations to a single zone.
>
> Fair enough, but that's not what this patch does. It's only for CMA.
I'm only certain that the CMA reservation from the device-tree is not
working now.
and if you guys think that this is not only the CMA but also other
non-CMA reservations
should also have this restriction on the device-tree method. I can
change the patch the patch
as the followings.
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 366d8c3..7b8857d 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -31,11 +31,15 @@ static int reserved_mem_count;
#if defined(CONFIG_HAVE_MEMBLOCK)
#include <linux/memblock.h>
-int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
- phys_addr_t align, phys_addr_t start, phys_addr_t end, bool nomap,
- phys_addr_t *res_base)
+int __init __weak early_init_dt_alloc_reserved_memory_arch(unsigned long node,
+ phys_addr_t size, phys_addr_t align, phys_addr_t start, phys_addr_t end,
+ bool nomap, phys_addr_t *res_base)
{
phys_addr_t base;
+ phys_addr_t highmem_start;
+
+ highmem_start = __pa(high_memory - 1) + 1;
+
/*
* We use __memblock_alloc_base() because memblock_alloc_base()
* panic()s on allocation failure.
@@ -53,15 +57,29 @@ int __init __weak
early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
return -ENOMEM;
}
+ /*
+ * Sanity check for the reserved region:If the reserved region
+ * crosses the low/high memory boundary, try to fix it up and then
+ * fall back to allocate region from the low mememory space.
+ */
+
+ if (base < highmem_start && (base + size) > highmem_start) {
+ memblock_free(base, size);
+ base = memblock_alloc_range(size, align, start,
+ highmem_start, MEMBLOCK_NONE);
+ if (!base)
+ return -ENOMEM;
+ }
+
if you guys have good idea, please share or post your patch. This is
really an issue
that reserve memory from the device-tree is broken.
>
> Rob
^ permalink raw reply related
* [PATCH v3] i2c: designware: Cleaning and comment style fixes.
From: Luis Oliveira @ 2016-12-15 15:09 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
The purpose of this commit is to fix some comments and styling issues in the
existing code due to the need of reuse this code. What is being made here is:
- Sorted the headers files
- Corrected some comments style
- Reverse tree in the variables declaration
- Add/remove empty lines and tabs where needed
- Fix of a misspelled word
The value of this, besides the rules of coding style, is because I
will use this code after and it will make my future patch a lot bigger and
complicated to review. The work here won't bring any additional work to
backported fixes because is just style and reordering.
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
---
Changes V2->V3: (Andy Shevchenko)
- Added the answers in the commit message.
- Added a comma, it seems to me it is the same sentence.
drivers/i2c/busses/i2c-designware-core.c | 36 ++++++++++++++---------------
drivers/i2c/busses/i2c-designware-core.h | 2 +-
drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++-----------
3 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 6d81c56184d3..9d724a6a7ec8 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -21,17 +21,17 @@
* ----------------------------------------------------------------------------
*
*/
+#include <linux/delay.h>
#include <linux/export.h>
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/io.h>
-#include <linux/pm_runtime.h>
-#include <linux/delay.h>
#include <linux/module.h>
-#include "i2c-designware-core.h"
+#include <linux/pm_runtime.h>
+#include "i2c-designware-core.h"
/*
* Registers offset
*/
@@ -98,7 +98,7 @@
#define DW_IC_ERR_TX_ABRT 0x1
-#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
+#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
#define DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH (BIT(2) | BIT(3))
#define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK GENMASK(3, 2)
@@ -113,10 +113,10 @@
#define TIMEOUT 20 /* ms */
/*
- * hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
+ * Hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
*
- * only expected abort codes are listed here
- * refer to the datasheet for the full list
+ * Only expected abort codes are listed here,
+ * refer to the datasheet for the full list.
*/
#define ABRT_7B_ADDR_NOACK 0
#define ABRT_10ADDR1_NOACK 1
@@ -338,14 +338,14 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
- /* Configure register endianess access */
+ /* Configure register endianness access */
dev->accessor_flags |= ACCESS_SWAP;
} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
/* Configure register access mode 16bit */
dev->accessor_flags |= ACCESS_16BIT;
} else if (reg != DW_IC_COMP_TYPE_VALUE) {
- dev_err(dev->dev, "Unknown Synopsys component type: "
- "0x%08x\n", reg);
+ dev_err(dev->dev,
+ "Unknown Synopsys component type: 0x%08x\n", reg);
i2c_dw_release_lock(dev);
return -ENODEV;
}
@@ -355,7 +355,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
/* Disable the adapter */
__i2c_dw_enable_and_wait(dev, false);
- /* set standard and fast speed deviders for high/low periods */
+ /* Set standard and fast speed deviders for high/low periods */
sda_falling_time = dev->sda_falling_time ?: 300; /* ns */
scl_falling_time = dev->scl_falling_time ?: 300; /* ns */
@@ -444,7 +444,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
dw_writel(dev, 0, DW_IC_RX_TL);
- /* configure the i2c master */
+ /* Configure the I2C master */
dw_writel(dev, dev->master_cfg , DW_IC_CON);
i2c_dw_release_lock(dev);
@@ -480,7 +480,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
/* Disable the adapter */
__i2c_dw_enable_and_wait(dev, false);
- /* if the slave address is ten bit address, enable 10BITADDR */
+ /* If the slave address is ten bit address, enable 10BITADDR */
if (dev->dynamic_tar_update_enabled) {
/*
* If I2C_DYNAMIC_TAR_UPDATE is set, the 10-bit addressing
@@ -505,7 +505,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
*/
dw_writel(dev, msgs[dev->msg_write_idx].addr | ic_tar, DW_IC_TAR);
- /* enforce disabled interrupts (due to HW issues) */
+ /* Enforce disabled interrupts (due to HW issues) */
i2c_dw_disable_int(dev);
/* Enable the adapter */
@@ -539,7 +539,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
u32 flags = msgs[dev->msg_write_idx].flags;
/*
- * if target address has changed, we need to
+ * If target address has changed, we need to
* reprogram the target address in the i2c
* adapter when we are done with this transfer
*/
@@ -601,7 +601,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
- /* avoid rx buffer overrun */
+ /* Avoid rx buffer overrun */
if (dev->rx_outstanding >= dev->rx_fifo_depth)
break;
@@ -905,7 +905,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
/*
* Anytime TX_ABRT is set, the contents of the tx/rx
- * buffers are flushed. Make sure to skip them.
+ * buffers are flushed. Make sure to skip them.
*/
dw_writel(dev, 0, DW_IC_INTR_MASK);
goto tx_aborted;
@@ -927,7 +927,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
complete(&dev->cmd_complete);
else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
- /* workaround to trigger pending interrupt */
+ /* Workaround to trigger pending interrupt */
stat = dw_readl(dev, DW_IC_INTR_MASK);
i2c_dw_disable_int(dev);
dw_writel(dev, stat, DW_IC_INTR_MASK);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 26250b425e2f..3cb81fca7738 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -36,7 +36,7 @@
#define DW_IC_CON_SPEED_FAST 0x4
#define DW_IC_CON_SPEED_HIGH 0x6
#define DW_IC_CON_SPEED_MASK 0x6
-#define DW_IC_CON_10BITADDR_MASTER 0x10
+#define DW_IC_CON_10BITADDR_MASTER 0x10
#define DW_IC_CON_RESTART_EN 0x20
#define DW_IC_CON_SLAVE_DISABLE 0x40
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 08153ea4d848..886e39753fc2 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -21,26 +21,27 @@
* ----------------------------------------------------------------------------
*
*/
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/dmi.h>
-#include <linux/i2c.h>
-#include <linux/clk.h>
-#include <linux/clk-provider.h>
-#include <linux/errno.h>
-#include <linux/sched.h>
#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/of.h>
+#include <linux/platform_data/i2c-designware.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
-#include <linux/io.h>
+#include <linux/sched.h>
#include <linux/slab.h>
-#include <linux/acpi.h>
-#include <linux/platform_data/i2c-designware.h>
+
#include "i2c-designware-core.h"
static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
@@ -153,11 +154,11 @@ static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
static int dw_i2c_plat_probe(struct platform_device *pdev)
{
struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
- struct dw_i2c_dev *dev;
struct i2c_adapter *adap;
+ struct dw_i2c_dev *dev;
+ u32 acpi_speed, ht = 0;
struct resource *mem;
int irq, r;
- u32 acpi_speed, ht = 0;
irq = platform_get_irq(pdev, 0);
if (irq < 0)
@@ -354,7 +355,7 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
#define DW_I2C_DEV_PMOPS NULL
#endif
-/* work with hotplug and coldplug */
+/* Work with hotplug and coldplug */
MODULE_ALIAS("platform:i2c_designware");
static struct platform_driver dw_i2c_driver = {
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 1/2] devicetree: power: add bindings for GPIO-driven power switches
From: Rob Herring @ 2016-12-15 15:05 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Mark Rutland,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-devicetree, LKML, Kevin Hilman, Patrick Titiano,
Neil Armstrong, Linus Walleij, Alexandre Courbot, linux-gpio,
Sebastian Reichel, linux-pm, Mark Brown, Liam Girdwood
In-Reply-To: <CAMpxmJWw3wCE4ch8TVik+2b3BC8Lvxbi13HGd9GxruSRLu95Mg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Dec 14, 2016 at 10:58 AM, Bartosz Golaszewski
<bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> 2016-12-13 20:27 GMT+01:00 Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
>> On Sun, Dec 11, 2016 at 11:21:44PM +0100, Bartosz Golaszewski wrote:
>>> Some boards are equipped with simple, GPIO-driven power load switches.
>>> An example of such ICs is the TI tps229* series.
>>
>> How is this different than a GPIO regulator? The input and output
>> voltages just happen to be the same. I could be convinced this is
>> different enough to have a different compatible, but it somewhat seems
>> you want to use this for IIO, so you are creating a different binding
>> for that usecase.
>>
>
> It's more of a fixed regulator I suppose. Do you mean adding a new
> compatible to the fixed-regulator binding (e.g. "gpio-power-switch" or
> "simple-power-switch") and then providing an iio driver for toggling
> the switch?
Yes, at least the first part. I view the switch as just a more
specific subtype of a fixed-regulator. Whether an IIO driver is a
separate discussion which is happening.
Rob
P.S. I really don't like compatibles with "simple".
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] of/platform: depopulate devices in the reverse order of creation
From: Rob Herring @ 2016-12-15 14:57 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Frank Rowand, Pawel Moll, Grant Likely,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20161214214012.GB6947@obsidianresearch.com>
On Wed, Dec 14, 2016 at 3:40 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote:
> On Wed, Dec 14, 2016 at 02:54:02PM -0600, Rob Herring wrote:
>> On Mon, Dec 12, 2016 at 12:39 PM, Jason Gunthorpe
>> <jgunthorpe@obsidianresearch.com> wrote:
>> > If the DT has inter-dependencies, then the devices need to be removed
>> > in the right order to avoid removal problems.
>> >
>> > Assuming the DT is constructed so that EPROBE_DEFER doesn't happen
>> > during creating then a good way to avoid removal problems is reversing
>> > the order during depopulation.
>>
>> I assume you mean by sorting the nodes to get lucky with the init
>> order.
>
> Not quite, the init order doesn't matter, just that the device list/DT
> is ordered topologically. The driver bind order can still be
> randomized. No luck needed.
I meant init in a generic sense
>
>> > In my specific case I have a gpio driver, followed by a i2c bitbang
>> > using that driver. So gpiolib prints an error if it the gpio driver is
>> > removed before the gpio client..
>>
>> Good news, functional dependencies are going into 4.10. Let's fix GPIO
>> and use that.
>
> Sure, but it will be some time until that is used everwhere..
>
> You don't think there is some value in 'hiding' the problem with
> ordering until that work is done? IIRC that was essentially how
> EPROBE_DEFER was introduced?
>
> Jason
^ permalink raw reply
* Re: [PATCH v2] i2c: designware: Cleaning and comment style fixes.
From: Andy Shevchenko @ 2016-12-15 14:56 UTC (permalink / raw)
To: Luis Oliveira, wsa, robh+dt, mark.rutland, jarkko.nikula,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
In-Reply-To: <d7b5a22ce09539c2af343c21ca69675bef6a0a10.1481812563.git.lolivei@synopsys.com>
On Thu, 2016-12-15 at 14:38 +0000, Luis Oliveira wrote:
> The purpose of this commit is to fix some comments and styling issues
> in the
> existing code due to the need of reuse this code. What is being made
> here is:
>
> - Sorted the headers files
> - Corrected some comments style
> - Reverse tree in the variables declaration
> - Add/remove empty lines and tabs where needed
> - Fix of a misspelled word
>
So, I'm okay with the change as long as no-one objecting it. As I
mentioned earlier it might rise concerns and better if you put the
answers to the commit message.
Also, comment below.
> @@ -113,10 +113,10 @@
> #define TIMEOUT 20 /* ms */
>
> /*
> - * hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
> + * Hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
> *
> - * only expected abort codes are listed here
> - * refer to the datasheet for the full list
> + * Only expected abort codes are listed here
> + * refer to the datasheet for the full list.
Feels either comma is missing, or they are two different sentences.
> */
> #define ABRT_7B_ADDR_NOACK 0
> #define ABRT_10ADDR1_NOACK 1
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* [GIT PULL] DeviceTree updates for 4.10
From: Rob Herring @ 2016-12-15 14:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: Frank Rowand, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Hi Linus,
Please pull. There's a trivial conflict in vendor-prefixes.txt. The
resolution is take both lines, but 'nvd' is also not sorted correctly
and it should come after 'nuvoton'.
Rob
The following changes since commit bc33b0ca11e3df467777a4fa7639ba488c9d4911:
Linux 4.9-rc4 (2016-11-05 16:23:36 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
tags/devicetree-for-4.10
for you to fetch changes up to 61eb3a04f9789c5e4f7498a2f171b82a567cea6b:
dt: pwm: bcm2835: fix typo in clocks property name (2016-12-09 15:37:58 -0600)
----------------------------------------------------------------
DeviceTree updates for 4.10:
- Add various vendor prefixes.
- Fix NUMA node handling when "numa=off" is passed on kernel command
line.
- Coding style Clean-up of overlay handling code.
- DocBook fixes in DT platform driver code
- Altera SoCFPGA binding addtions for freeze bridge, arria10 FPGA
manager and FPGA bridges.
- Couple of printk message fixes.
----------------------------------------------------------------
Alan Tull (3):
ARM: socfpga: add bindings document for fpga bridge drivers
ARM: socfpga: add bindings doc for arria10 fpga manager
add bindings document for altera freeze bridge
David Daney (1):
of, numa: Return NUMA_NO_NODE from disable of_node_to_nid() if
nid not possible.
Dinh Nguyen (3):
dt-bindings: Add Macnica Americas vendor prefix
dt-bindings: Add vendor prefix for Terasic Inc.
dt-bindings: Add vendor prefix for Samtec
Fabio Estevam (1):
dt-bindings: Add vendor prefix for Udoo
Frank Rowand (12):
of: Remove comments that state the obvious, to reduce clutter
of: Remove excessive printks to reduce clutter.
of: Convert comparisons to zero or NULL to logical expressions
of: Rename functions to more accurately reflect what they do
of: Remove prefix "__of_" from local function names
of: Rename variables to better reflect purpose or follow convention
of: Update structure of code to be clearer, also remove BUG_ON()
of: Remove redundant size check
of: Update comments to reflect changes and increase clarity
of: Add back an error message, restructured
of: Move setting of pointer to beside test for non-null
of: Remove unused variable overlay_symbols
Geert Uytterhoeven (2):
dt/bindings: arm-boards: Remove skeleton.dtsi inclusion from example
devicetree: bindings: Add vendor prefix for Oki
Greentime Hu (1):
devicetree: bindings: Add vendor prefix for Andes Technology Corporation
Johan Hovold (2):
of/platform: fix of_platform_device_destroy comment
of/platform: clarify of_find_device_by_node refcounting
Marcin Nowakowski (1):
drivers/of: fix missing pr_cont()s in of_print_phandle_args
Marek Vasut (1):
of: Add vendor prefix for Aries Embedded GmbH
Moritz Fischer (1):
of: Fix issue where code would fall through to error case.
Nathan Sullivan (1):
devicetree: add vendor prefix for National Instruments
Rob Herring (1):
Revert "of: base: add support to get machine model name"
Sudeep Holla (1):
of: base: add support to get machine model name
Vladimir Zapolskiy (2):
dt-bindings: add MYIR Tech hardware vendor prefix
dt: pwm: bcm2835: fix typo in clocks property name
Documentation/devicetree/bindings/arm/arm-boards | 3 +-
.../bindings/fpga/altera-fpga2sdram-bridge.txt | 16 +
.../bindings/fpga/altera-freeze-bridge.txt | 23 ++
.../bindings/fpga/altera-hps2fpga-bridge.txt | 39 +++
.../bindings/fpga/altera-socfpga-a10-fpga-mgr.txt | 19 ++
.../devicetree/bindings/pwm/pwm-bcm2835.txt | 2 +-
.../devicetree/bindings/vendor-prefixes.txt | 9 +
drivers/of/base.c | 9 +-
drivers/of/of_numa.c | 7 +-
drivers/of/platform.c | 6 +-
drivers/of/resolver.c | 358 +++++++++------------
11 files changed, 277 insertions(+), 214 deletions(-)
create mode 100644
Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt
create mode 100644
Documentation/devicetree/bindings/fpga/altera-freeze-bridge.txt
create mode 100644
Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt
create mode 100644
Documentation/devicetree/bindings/fpga/altera-socfpga-a10-fpga-mgr.txt
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] i2c: designware: Cleaning and comment style fixes.
From: Luis Oliveira @ 2016-12-15 14:38 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
The purpose of this commit is to fix some comments and styling issues in the
existing code due to the need of reuse this code. What is being made here is:
- Sorted the headers files
- Corrected some comments style
- Reverse tree in the variables declaration
- Add/remove empty lines and tabs where needed
- Fix of a misspelled word
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
---
Changes V1->V2: (Andy Shevchenko)
- Removed all the unneeded dots as suggested
- I checked and "acknowledgement" and "acknowledgment"" are both correct
drivers/i2c/busses/i2c-designware-core.c | 36 ++++++++++++++---------------
drivers/i2c/busses/i2c-designware-core.h | 2 +-
drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++++++++++-----------
3 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 6d81c56184d3..254d82bb6bc3 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -21,17 +21,17 @@
* ----------------------------------------------------------------------------
*
*/
+#include <linux/delay.h>
#include <linux/export.h>
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/io.h>
-#include <linux/pm_runtime.h>
-#include <linux/delay.h>
#include <linux/module.h>
-#include "i2c-designware-core.h"
+#include <linux/pm_runtime.h>
+#include "i2c-designware-core.h"
/*
* Registers offset
*/
@@ -98,7 +98,7 @@
#define DW_IC_ERR_TX_ABRT 0x1
-#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
+#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
#define DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH (BIT(2) | BIT(3))
#define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK GENMASK(3, 2)
@@ -113,10 +113,10 @@
#define TIMEOUT 20 /* ms */
/*
- * hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
+ * Hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
*
- * only expected abort codes are listed here
- * refer to the datasheet for the full list
+ * Only expected abort codes are listed here
+ * refer to the datasheet for the full list.
*/
#define ABRT_7B_ADDR_NOACK 0
#define ABRT_10ADDR1_NOACK 1
@@ -338,14 +338,14 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
- /* Configure register endianess access */
+ /* Configure register endianness access */
dev->accessor_flags |= ACCESS_SWAP;
} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
/* Configure register access mode 16bit */
dev->accessor_flags |= ACCESS_16BIT;
} else if (reg != DW_IC_COMP_TYPE_VALUE) {
- dev_err(dev->dev, "Unknown Synopsys component type: "
- "0x%08x\n", reg);
+ dev_err(dev->dev,
+ "Unknown Synopsys component type: 0x%08x\n", reg);
i2c_dw_release_lock(dev);
return -ENODEV;
}
@@ -355,7 +355,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
/* Disable the adapter */
__i2c_dw_enable_and_wait(dev, false);
- /* set standard and fast speed deviders for high/low periods */
+ /* Set standard and fast speed deviders for high/low periods */
sda_falling_time = dev->sda_falling_time ?: 300; /* ns */
scl_falling_time = dev->scl_falling_time ?: 300; /* ns */
@@ -444,7 +444,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
dw_writel(dev, 0, DW_IC_RX_TL);
- /* configure the i2c master */
+ /* Configure the I2C master */
dw_writel(dev, dev->master_cfg , DW_IC_CON);
i2c_dw_release_lock(dev);
@@ -480,7 +480,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
/* Disable the adapter */
__i2c_dw_enable_and_wait(dev, false);
- /* if the slave address is ten bit address, enable 10BITADDR */
+ /* If the slave address is ten bit address, enable 10BITADDR */
if (dev->dynamic_tar_update_enabled) {
/*
* If I2C_DYNAMIC_TAR_UPDATE is set, the 10-bit addressing
@@ -505,7 +505,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
*/
dw_writel(dev, msgs[dev->msg_write_idx].addr | ic_tar, DW_IC_TAR);
- /* enforce disabled interrupts (due to HW issues) */
+ /* Enforce disabled interrupts (due to HW issues) */
i2c_dw_disable_int(dev);
/* Enable the adapter */
@@ -539,7 +539,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
u32 flags = msgs[dev->msg_write_idx].flags;
/*
- * if target address has changed, we need to
+ * If target address has changed, we need to
* reprogram the target address in the i2c
* adapter when we are done with this transfer
*/
@@ -601,7 +601,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
- /* avoid rx buffer overrun */
+ /* Avoid rx buffer overrun */
if (dev->rx_outstanding >= dev->rx_fifo_depth)
break;
@@ -905,7 +905,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
/*
* Anytime TX_ABRT is set, the contents of the tx/rx
- * buffers are flushed. Make sure to skip them.
+ * buffers are flushed. Make sure to skip them.
*/
dw_writel(dev, 0, DW_IC_INTR_MASK);
goto tx_aborted;
@@ -927,7 +927,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
complete(&dev->cmd_complete);
else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
- /* workaround to trigger pending interrupt */
+ /* Workaround to trigger pending interrupt */
stat = dw_readl(dev, DW_IC_INTR_MASK);
i2c_dw_disable_int(dev);
dw_writel(dev, stat, DW_IC_INTR_MASK);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 26250b425e2f..3cb81fca7738 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -36,7 +36,7 @@
#define DW_IC_CON_SPEED_FAST 0x4
#define DW_IC_CON_SPEED_HIGH 0x6
#define DW_IC_CON_SPEED_MASK 0x6
-#define DW_IC_CON_10BITADDR_MASTER 0x10
+#define DW_IC_CON_10BITADDR_MASTER 0x10
#define DW_IC_CON_RESTART_EN 0x20
#define DW_IC_CON_SLAVE_DISABLE 0x40
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 08153ea4d848..886e39753fc2 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -21,26 +21,27 @@
* ----------------------------------------------------------------------------
*
*/
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/dmi.h>
-#include <linux/i2c.h>
-#include <linux/clk.h>
-#include <linux/clk-provider.h>
-#include <linux/errno.h>
-#include <linux/sched.h>
#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/of.h>
+#include <linux/platform_data/i2c-designware.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
-#include <linux/io.h>
+#include <linux/sched.h>
#include <linux/slab.h>
-#include <linux/acpi.h>
-#include <linux/platform_data/i2c-designware.h>
+
#include "i2c-designware-core.h"
static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
@@ -153,11 +154,11 @@ static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
static int dw_i2c_plat_probe(struct platform_device *pdev)
{
struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
- struct dw_i2c_dev *dev;
struct i2c_adapter *adap;
+ struct dw_i2c_dev *dev;
+ u32 acpi_speed, ht = 0;
struct resource *mem;
int irq, r;
- u32 acpi_speed, ht = 0;
irq = platform_get_irq(pdev, 0);
if (irq < 0)
@@ -354,7 +355,7 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
#define DW_I2C_DEV_PMOPS NULL
#endif
-/* work with hotplug and coldplug */
+/* Work with hotplug and coldplug */
MODULE_ALIAS("platform:i2c_designware");
static struct platform_driver dw_i2c_driver = {
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 2/2] xilinx_dma: Add reset support
From: Laurent Pinchart @ 2016-12-15 13:56 UTC (permalink / raw)
To: Ramiro Oliveira
Cc: dmaengine-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
vinod.koul-ral2JQCrhuEAvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, michal.simek-gjFFaj9aHVfQT0dZR+AlfA,
soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA,
appana.durga.rao-gjFFaj9aHVfQT0dZR+AlfA,
anuragku-gjFFaj9aHVfQT0dZR+AlfA,
dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w, Philipp Zabel
In-Reply-To: <380d1b11-814d-0164-3d49-e976f2deb3f9-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Hi Ramiro,
(CC'ing Philipp Zabel)
On Thursday 15 Dec 2016 11:26:54 Ramiro Oliveira wrote:
> On 12/14/2016 8:16 PM, Laurent Pinchart wrote:
> > Hi Ramiro,
> >
> > Thank you for the patch.
> >
> > On Wednesday 14 Dec 2016 17:18:24 Ramiro Oliveira wrote:
> >> Add a DT property to control an optional external reset line
> >>
> >> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >> ---
> >>
> >> drivers/dma/xilinx/xilinx_dma.c | 23 +++++++++++++++++++++++
> >> 1 file changed, 23 insertions(+)
> >>
> >> diff --git a/drivers/dma/xilinx/xilinx_dma.c
> >> b/drivers/dma/xilinx/xilinx_dma.c index 5c9f11b..b845224 100644
> >> --- a/drivers/dma/xilinx/xilinx_dma.c
> >> +++ b/drivers/dma/xilinx/xilinx_dma.c
> >> @@ -46,6 +46,7 @@
> >> #include <linux/slab.h>
> >> #include <linux/clk.h>
> >> #include <linux/io-64-nonatomic-lo-hi.h>
> >> +#include <linux/reset.h>
> >
> > I had neatly sorted the header alphabetically until someone added clk.h
> > and io-64-nonatomic-lo-hi.h :-( Could you please move reset.h just before
> > slab.h ?
>
> Sure. Actually I was tempted to reorder it, but I decided not to do it. I'll
> do it now
Yeah, I'll sleep better tonight :-D
> >> #include "../dmaengine.h"
> >>
> >> @@ -409,6 +410,7 @@ struct xilinx_dma_device {
> >> struct clk *rxs_clk;
> >> u32 nr_channels;
> >> u32 chan_id;
> >> + struct reset_control *rst;
> >> };
> >>
> >> /* Macros */
> >> @@ -2543,6 +2545,27 @@ static int xilinx_dma_probe(struct platform_device
> >> *pdev) if (IS_ERR(xdev->regs))
> >> return PTR_ERR(xdev->regs);
> >>
> >> + xdev->rst = devm_reset_control_get_optional(&pdev->dev, "reset");
> >
> > devm_reset_control_get_optional() is deprecated as explained in
> > linux/reset.h, you should use devm_reset_control_get_optional_exclusive()
> > or devm_reset_control_get_optional_shared() instead, as applicable.
> >
> > This being said, I'm wondering why the optional versions of those
> > functions exist, as they do exactly the same as the non-optional versions.
> > The API feels wrong, it should have been modelled like the GPIO API. Feel
> > free to fix it if you want :-) But that's out of scope for this patch.
>
> I missed the comment stating that devm_reset_control_get_optional() was
> deprecated.
>
> I could fix it. Your sugestion is modelling these functions like the GPIO
> API?
I think it would be better for driver if the _get_optional functions would
return an ERR_PTR() for errors and NULL when reset control is not available,
and then have the rest of the reset controller API accept NULL as a no-op.
Your implementation would then be
xdev->rst = devm_reset_control_get_optional_exclusive(&pdev->dev,
"reset");
if (IS_ERR(xdev->rst)) {
err = PTR_ERR(xdev->rst);
if (err != -EPROBE_DEFER)
dev_err(xdev->dev, "error getting reset %d\n", err);
return err;
}
err = reset_control_deassert(xdev->rst);
if (err) {
dev_err(xdev->dev, "failed to deassert reset: %d\n", err);
return err;
}
That requires modifying the reset controller API, so it's a bit out-of-scope,
but if you could give it a go, it would be great.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: Add missing CPU frequencies for Exynos5422/5800
From: Markus Reichl @ 2016-12-15 13:55 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski
Cc: Kukjin Kim, Javier Martinez Canillas, Rob Herring, Mark Rutland,
Russell King, Doug Anderson, Andreas Faerber, Thomas Abraham,
Ben Gamari, Arjun K V, linux-samsung-soc, linux-arm-kernel,
linux-pm, devicetree, linux-kernel
In-Reply-To: <10512254.nyUcL0zgTP@amdc3058>
Hi Bartlomiej,
Am 15.12.2016 um 12:55 schrieb Bartlomiej Zolnierkiewicz:
> Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
> (for A7 cores). Also update common Odroid-XU3 Lite/XU3/XU4 thermal
> cooling maps to account for new OPPs.
>
> Since new OPPs are not available on all Exynos5422/5800 boards modify
> dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
> Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
>
> Tested on Odroid-XU3 and XU3 Lite.
>
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Andreas Faerber <afaerber@suse.de>
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> Cc: Ben Gamari <ben@smart-cactus.org>
> Cc: Arjun K V <arjun.kv@samsung.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
> v2:
> - added comments about limitations of SoC revisions used by Odroid-XU3 Lite and
> Peach Pi boards (suggested by Javier)
> - removed redundant opp_a7_14 label
> - added Arjun to Cc:
>
> Javier, could you test it on Peach Pi board?
>
> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 14 ++++++-------
> arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts | 22 +++++++++++++++++++++
> arch/arm/boot/dts/exynos5800-peach-pi.dts | 9 ++++++++
> arch/arm/boot/dts/exynos5800.dtsi | 15 ++++++++++++++
> 4 files changed, 53 insertions(+), 7 deletions(-)
>
> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi 2016-12-15 12:43:54.365955950 +0100
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi 2016-12-15 12:43:54.361955949 +0100
> @@ -118,7 +118,7 @@
> /*
> * When reaching cpu_alert3, reduce CPU
> * by 2 steps. On Exynos5422/5800 that would
> - * be: 1600 MHz and 1100 MHz.
> + * (usually) be: 1800 MHz and 1200 MHz.
> */
> map3 {
> trip = <&cpu_alert3>;
> @@ -131,16 +131,16 @@
>
> /*
> * When reaching cpu_alert4, reduce CPU
> - * further, down to 600 MHz (11 steps for big,
> - * 7 steps for LITTLE).
> + * further, down to 600 MHz (13 steps for big,
> + * 8 steps for LITTLE).
> */
> - map5 {
> + cooling_map5: map5 {
> trip = <&cpu_alert4>;
> - cooling-device = <&cpu0 3 7>;
> + cooling-device = <&cpu0 3 8>;
> };
> - map6 {
> + cooling_map6: map6 {
> trip = <&cpu_alert4>;
> - cooling-device = <&cpu4 3 11>;
> + cooling-device = <&cpu4 3 13>;
> };
> };
> };
> Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts 2016-12-15 12:43:54.365955950 +0100
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts 2016-12-15 12:43:54.361955949 +0100
> @@ -21,6 +21,28 @@
> compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
> };
>
> +/*
> + * Odroid XU3-Lite board uses SoC revision with lower maximum frequencies
> + * than Odroid XU3/XU4 boards: 1.8 GHz for A15 cores & 1.3 GHz for A7 cores.
> + * Therefore we need to update OPPs tables and thermal maps accordingly.
> + */
> +&cluster_a15_opp_table {
> + /delete-node/opp@2000000000;
> + /delete-node/opp@1900000000;
> +};
> +
> +&cluster_a7_opp_table {
> + /delete-node/opp@1400000000;
> +};
> +
> +&cooling_map5 {
> + cooling-device = <&cpu0 3 7>;
> +};
> +
> +&cooling_map6 {
> + cooling-device = <&cpu4 3 11>;
> +};
> +
> &pwm {
> /*
> * PWM 0 -- fan
> Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts 2016-12-15 12:43:54.365955950 +0100
> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts 2016-12-15 12:43:54.361955949 +0100
> @@ -146,6 +146,15 @@
> vdd-supply = <&ldo9_reg>;
> };
>
> +/*
> + * Peach Pi board uses SoC revision with lower maximum frequency for A7 cores
> + * (1.3 GHz instead of 1.4 GHz) than Odroid XU3/XU4 boards. Thus we need to
> + * update A7 OPPs table accordingly.
> + */
> +&cluster_a7_opp_table {
> + /delete-property/opp@1400000000;
> +};
> +
> &cpu0 {
> cpu-supply = <&buck2_reg>;
> };
> Index: b/arch/arm/boot/dts/exynos5800.dtsi
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5800.dtsi 2016-12-15 12:43:54.365955950 +0100
> +++ b/arch/arm/boot/dts/exynos5800.dtsi 2016-12-15 12:43:54.361955949 +0100
> @@ -24,6 +24,16 @@
> };
>
> &cluster_a15_opp_table {
> + opp@2000000000 {
> + opp-hz = /bits/ 64 <2000000000>;
> + opp-microvolt = <1250000>;
> + clock-latency-ns = <140000>;
> + };
> + opp@1900000000 {
> + opp-hz = /bits/ 64 <1900000000>;
> + opp-microvolt = <1250000>;
> + clock-latency-ns = <140000>;
> + };
> opp@1700000000 {
> opp-microvolt = <1250000>;
> };
> @@ -85,6 +95,11 @@
> };
>
> &cluster_a7_opp_table {
> + opp@1400000000 {
> + opp-hz = /bits/ 64 <1400000000>;
> + opp-microvolt = <1250000>;
> + clock-latency-ns = <140000>;
> + };
> opp@1300000000 {
> opp-microvolt = <1250000>;
> };
>
On Odroid XU4, XU3 and XU3-lite:
Tested-by: Markus Reichl <m.reichl@fivetechno.de>
Thanks,
--
Markus Reichl
^ permalink raw reply
* Re: [PATCH 1/1] of: of_reserved_mem: Ensure cma reserved region not cross the low/high memory
From: Rob Herring @ 2016-12-15 13:54 UTC (permalink / raw)
To: Laura Abbott
Cc: Jason Liu, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Frank Rowand
In-Reply-To: <833f99b1-d10b-27ba-e0f9-a7c6398fb63d-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Wed, Dec 14, 2016 at 4:21 PM, Laura Abbott <labbott-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 12/14/2016 12:45 PM, Rob Herring wrote:
>> On Wed, Nov 23, 2016 at 5:37 AM, Jason Liu <jason.hui.liu-3arQi8VN3Tc@public.gmane.org> wrote:
>>> Need ensure the cma reserved region not cross the low/high memory boundary
>>> when using the dynamic allocation methond through device-tree, otherwise,
>>> kernel will fail to boot up when cma reserved region cross how/high mem.
>>
>> The kernel command line code setting CMA already deals with this. Why
>> don't we just call the CMA code (cma_declare_contiguous) to deal with
>> this?
>>
>> Rob
>>
>
> That was proposed in the first version[1] but I think this is a generic
> problem not specific to CMA. Even non-CMA reservations trying to span
> zones could cause problems so the devicetree allocation code should
> restrict reservations to a single zone.
Fair enough, but that's not what this patch does. It's only for CMA.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RESEND PATCHv3 2/2] regulator: fixed: Handle optional overcurrent pin
From: Axel Haslam @ 2016-12-15 13:03 UTC (permalink / raw)
To: Mark Brown
Cc: Liam Girdwood, Kevin Hilman, Sekhar Nori, David Lechner,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161215121958.klssvbtbuhsz3pga-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
On Thu, Dec 15, 2016 at 1:19 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Dec 15, 2016 at 12:28:56PM +0100, Axel Haslam wrote:
>
>> + ret = devm_request_threaded_irq(&pdev->dev,
>> + gpiod_to_irq(drvdata->oc_gpio), NULL,
>> + reg_fixed_overcurrent_irq, irqflags,
>> + "over_current", drvdata);
>
>> drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
>> &cfg);
>
> We are registering a managed interrupt for the interrupt handler and we
> are registering it before we register the regulator. This means that
> the interrupt may fire in both probe and remove paths without the
> regulator which it will then try to use in the interrupt handler,
> potentially crashing the system. It's better to register the interrupt
> after the regulator (which will make the managed bit OK) to avoid this
> possibility.
will fix, and send v4.
Regards,
Axel.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3] dt-bindings: power: supply: bq24735: reverse the polarity of ac-detect
From: Peter Rosin @ 2016-12-15 12:21 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
linux-pm, devicetree, linux-tegra, Jon Hunter
The ACOK pin on the bq24735 is active-high, of course meaning that when
AC is OK the pin is high. However, all Tegra dts files have incorrectly
specified active-high even though the signal is inverted on the Tegra
boards. This has worked since the Linux driver has also inverted the
meaning of the GPIO. Fix this situation by simply specifying in the
bindings what everybody else agrees on; that the ti,ac-detect-gpios is
active on AC adapter absence.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Hi!
This patch is the result of this discussion:
http://marc.info/?t=148152531800002
I don't like how it changes the one thing that is seems correct, but
what to do?
Cheers,
Peter
Documentation/devicetree/bindings/power/supply/ti,bq24735.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
index 3bf55757ceec..43d56c49455b 100644
--- a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
+++ b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
@@ -8,8 +8,9 @@ Optional properties :
- interrupts : Specify the interrupt to be used to trigger when the AC
adapter is either plugged in or removed.
- ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
- presence. This is a Host GPIO that is configured as an input and
- connected to the bq24735.
+ status. This is a Host GPIO that is configured as an input and
+ connected to the bq24735, typically the ACOK pin (note: the GPIO should
+ be active on AC adapter absence).
- ti,charge-current : Used to control and set the charging current. This value
must be between 128mA and 8.128A with a 64mA step resolution. The POR value
is 0x0000h. This number is in mA (e.g. 8192), see spec for more information
--
2.1.4
^ permalink raw reply related
* Re: [PATCH 0/3] clkdev: add devm_get_clk_from_child()
From: Mark Brown @ 2016-12-15 12:21 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Stephen Boyd, Russell King - ARM Linux, Rob Herring, Linux-ALSA,
Linux-DT, Michael Turquette, Linux-Kernel, linux-clk, Linux-ARM
In-Reply-To: <8737hyymo9.wl%kuninori.morimoto.gx@renesas.com>
[-- Attachment #1: Type: text/plain, Size: 291 bytes --]
On Fri, Dec 09, 2016 at 12:25:48AM +0000, Kuninori Morimoto wrote:
> Mark, I think I should re-post 2nd patch (3rd will be dropped) after
> merge window ? There will be no branch dependency
Should be fine without but obviously if it looks like it's gone AWOL
then reposting would be good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Applied "misc: atmel-ssc: register as sound DAI if #sound-dai-cells is present" to the asoc tree
From: Mark Brown @ 2016-12-15 12:20 UTC (permalink / raw)
To: Peter Rosin
Cc: Rob Herring, Nicolas Ferre, Mark Brown, linux-kernel,
Mark Rutland, devicetree, alsa-devel, Arnd Bergmann,
Greg Kroah-Hartman, Takashi Iwai
In-Reply-To: <1480593549-6464-2-git-send-email-peda@axentia.se>
The patch
misc: atmel-ssc: register as sound DAI if #sound-dai-cells is present
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From e8314d7d53c8b050aac2828a5de5f28a997b468b Mon Sep 17 00:00:00 2001
From: Peter Rosin <peda@axentia.se>
Date: Tue, 6 Dec 2016 20:22:36 +0100
Subject: [PATCH] misc: atmel-ssc: register as sound DAI if #sound-dai-cells is
present
The SSC is currently not usable with the ASoC simple-audio-card, as
every SSC audio user has to build a platform driver that may do as
little as calling atmel_ssc_set_audio/atmel_ssc_put_audio (which
allocates the SSC and registers a DAI with the ASoC subsystem).
So, have that happen automatically, if the #sound-dai-cells property
is present in devicetree, which it has to be anyway for simple audio
card to work.
Signed-off-by: Peter Rosin <peda@axentia.se>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
.../devicetree/bindings/misc/atmel-ssc.txt | 2 +
drivers/misc/atmel-ssc.c | 50 ++++++++++++++++++++++
include/linux/atmel-ssc.h | 1 +
3 files changed, 53 insertions(+)
diff --git a/Documentation/devicetree/bindings/misc/atmel-ssc.txt b/Documentation/devicetree/bindings/misc/atmel-ssc.txt
index efc98ea1f23d..f8629bb73945 100644
--- a/Documentation/devicetree/bindings/misc/atmel-ssc.txt
+++ b/Documentation/devicetree/bindings/misc/atmel-ssc.txt
@@ -24,6 +24,8 @@ Optional properties:
this parameter to choose where the clock from.
- By default the clock is from TK pin, if the clock from RK pin, this
property is needed.
+ - #sound-dai-cells: Should contain <0>.
+ - This property makes the SSC into an automatically registered DAI.
Examples:
- PDC transfer:
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
index 0516ecda54d3..b2a0340f277e 100644
--- a/drivers/misc/atmel-ssc.c
+++ b/drivers/misc/atmel-ssc.c
@@ -20,6 +20,8 @@
#include <linux/of.h>
+#include "../../sound/soc/atmel/atmel_ssc_dai.h"
+
/* Serialize access to ssc_list and user count */
static DEFINE_SPINLOCK(user_lock);
static LIST_HEAD(ssc_list);
@@ -145,6 +147,49 @@ static inline const struct atmel_ssc_platform_data * __init
platform_get_device_id(pdev)->driver_data;
}
+#ifdef CONFIG_SND_ATMEL_SOC_SSC
+static int ssc_sound_dai_probe(struct ssc_device *ssc)
+{
+ struct device_node *np = ssc->pdev->dev.of_node;
+ int ret;
+ int id;
+
+ ssc->sound_dai = false;
+
+ if (!of_property_read_bool(np, "#sound-dai-cells"))
+ return 0;
+
+ id = of_alias_get_id(np, "ssc");
+ if (id < 0)
+ return id;
+
+ ret = atmel_ssc_set_audio(id);
+ ssc->sound_dai = !ret;
+
+ return ret;
+}
+
+static void ssc_sound_dai_remove(struct ssc_device *ssc)
+{
+ if (!ssc->sound_dai)
+ return;
+
+ atmel_ssc_put_audio(of_alias_get_id(ssc->pdev->dev.of_node, "ssc"));
+}
+#else
+static inline int ssc_sound_dai_probe(struct ssc_device *ssc)
+{
+ if (of_property_read_bool(ssc->pdev->dev.of_node, "#sound-dai-cells"))
+ return -ENOTSUPP;
+
+ return 0;
+}
+
+static inline void ssc_sound_dai_remove(struct ssc_device *ssc)
+{
+}
+#endif
+
static int ssc_probe(struct platform_device *pdev)
{
struct resource *regs;
@@ -204,6 +249,9 @@ static int ssc_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "Atmel SSC device at 0x%p (irq %d)\n",
ssc->regs, ssc->irq);
+ if (ssc_sound_dai_probe(ssc))
+ dev_err(&pdev->dev, "failed to auto-setup ssc for audio\n");
+
return 0;
}
@@ -211,6 +259,8 @@ static int ssc_remove(struct platform_device *pdev)
{
struct ssc_device *ssc = platform_get_drvdata(pdev);
+ ssc_sound_dai_remove(ssc);
+
spin_lock(&user_lock);
list_del(&ssc->list);
spin_unlock(&user_lock);
diff --git a/include/linux/atmel-ssc.h b/include/linux/atmel-ssc.h
index 7c0f6549898b..fdb545101ede 100644
--- a/include/linux/atmel-ssc.h
+++ b/include/linux/atmel-ssc.h
@@ -20,6 +20,7 @@ struct ssc_device {
int user;
int irq;
bool clk_from_rk_pin;
+ bool sound_dai;
};
struct ssc_device * __must_check ssc_request(unsigned int ssc_num);
--
2.11.0
^ permalink raw reply related
* Applied "ASoC: atmel: tse850: rely on the ssc to register as a cpu dai by itself" to the asoc tree
From: Mark Brown @ 2016-12-15 12:20 UTC (permalink / raw)
To: Peter Rosin
Cc: Rob Herring, Mark Brown, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Arnd Bergmann,
Greg Kroah-Hartman, Takashi Iwai, Nicolas Ferre, Liam Girdwood,
Rob Herring
In-Reply-To: <1481052157-23400-3-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
The patch
ASoC: atmel: tse850: rely on the ssc to register as a cpu dai by itself
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From ca8c7f233fa2c40e2a23f982dc33d947f28ad207 Mon Sep 17 00:00:00 2001
From: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Date: Tue, 6 Dec 2016 20:22:37 +0100
Subject: [PATCH] ASoC: atmel: tse850: rely on the ssc to register as a cpu dai
by itself
This breaks devicetree compatibility, but in this case that is ok. All
affected units are either on my desk, or running an even older version
of the driver that is not compatible with the upstreamed version anyway
(and when these other units are eventually updated, they will get a
fresh dtb as well, so that is not a significant problem either).
All of that is of course assuming that noone else has managed to build
something that can use this driver, but that seems extremely improbable.
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
.../bindings/sound/axentia,tse850-pcm5142.txt | 11 ++++++++---
sound/soc/atmel/tse850-pcm5142.c | 23 +++-------------------
2 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt b/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt
index 5b9b38f578bb..fdb25b492514 100644
--- a/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt
+++ b/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt
@@ -2,8 +2,7 @@ Devicetree bindings for the Axentia TSE-850 audio complex
Required properties:
- compatible: "axentia,tse850-pcm5142"
- - axentia,ssc-controller: The phandle of the atmel SSC controller used as
- cpu dai.
+ - axentia,cpu-dai: The phandle of the cpu dai.
- axentia,audio-codec: The phandle of the PCM5142 codec.
- axentia,add-gpios: gpio specifier that controls the mixer.
- axentia,loop1-gpios: gpio specifier that controls loop relays on channel 1.
@@ -43,6 +42,12 @@ the PCM5142 codec.
Example:
+ &ssc0 {
+ #sound-dai-cells = <0>;
+
+ status = "okay";
+ };
+
&i2c {
codec: pcm5142@4c {
compatible = "ti,pcm5142";
@@ -77,7 +82,7 @@ Example:
sound {
compatible = "axentia,tse850-pcm5142";
- axentia,ssc-controller = <&ssc0>;
+ axentia,cpu-dai = <&ssc0>;
axentia,audio-codec = <&codec>;
axentia,add-gpios = <&pioA 8 GPIO_ACTIVE_LOW>;
diff --git a/sound/soc/atmel/tse850-pcm5142.c b/sound/soc/atmel/tse850-pcm5142.c
index ac6a814c8ecf..a72c7d642026 100644
--- a/sound/soc/atmel/tse850-pcm5142.c
+++ b/sound/soc/atmel/tse850-pcm5142.c
@@ -51,11 +51,7 @@
#include <sound/soc.h>
#include <sound/pcm_params.h>
-#include "atmel_ssc_dai.h"
-
struct tse850_priv {
- int ssc_id;
-
struct gpio_desc *add;
struct gpio_desc *loop1;
struct gpio_desc *loop2;
@@ -329,23 +325,20 @@ static int tse850_dt_init(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct device_node *codec_np, *cpu_np;
- struct snd_soc_card *card = &tse850_card;
struct snd_soc_dai_link *dailink = &tse850_dailink;
- struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
if (!np) {
dev_err(&pdev->dev, "only device tree supported\n");
return -EINVAL;
}
- cpu_np = of_parse_phandle(np, "axentia,ssc-controller", 0);
+ cpu_np = of_parse_phandle(np, "axentia,cpu-dai", 0);
if (!cpu_np) {
- dev_err(&pdev->dev, "failed to get dai and pcm info\n");
+ dev_err(&pdev->dev, "failed to get cpu dai\n");
return -EINVAL;
}
dailink->cpu_of_node = cpu_np;
dailink->platform_of_node = cpu_np;
- tse850->ssc_id = of_alias_get_id(cpu_np, "ssc");
of_node_put(cpu_np);
codec_np = of_parse_phandle(np, "axentia,audio-codec", 0);
@@ -415,23 +408,14 @@ static int tse850_probe(struct platform_device *pdev)
return ret;
}
- ret = atmel_ssc_set_audio(tse850->ssc_id);
- if (ret != 0) {
- dev_err(dev,
- "failed to set SSC %d for audio\n", tse850->ssc_id);
- goto err_disable_ana;
- }
-
ret = snd_soc_register_card(card);
if (ret) {
dev_err(dev, "snd_soc_register_card failed\n");
- goto err_put_audio;
+ goto err_disable_ana;
}
return 0;
-err_put_audio:
- atmel_ssc_put_audio(tse850->ssc_id);
err_disable_ana:
regulator_disable(tse850->ana);
return ret;
@@ -443,7 +427,6 @@ static int tse850_remove(struct platform_device *pdev)
struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
snd_soc_unregister_card(card);
- atmel_ssc_put_audio(tse850->ssc_id);
regulator_disable(tse850->ana);
return 0;
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [RESEND PATCHv3 2/2] regulator: fixed: Handle optional overcurrent pin
From: Mark Brown @ 2016-12-15 12:19 UTC (permalink / raw)
To: Axel Haslam
Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, khilman-rdvid1DuHRBWk0Htik3J/w,
nsekhar-l0cyMroinI0, david-nq/r/kbU++upp/zk7JDF2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161215112856.9541-3-ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
On Thu, Dec 15, 2016 at 12:28:56PM +0100, Axel Haslam wrote:
> + ret = devm_request_threaded_irq(&pdev->dev,
> + gpiod_to_irq(drvdata->oc_gpio), NULL,
> + reg_fixed_overcurrent_irq, irqflags,
> + "over_current", drvdata);
> drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
> &cfg);
We are registering a managed interrupt for the interrupt handler and we
are registering it before we register the regulator. This means that
the interrupt may fire in both probe and remove paths without the
regulator which it will then try to use in the interrupt handler,
potentially crashing the system. It's better to register the interrupt
after the regulator (which will make the managed bit OK) to avoid this
possibility.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: sunxi: Change node name for pwrseq pin on Olinuxino-lime2-emmc
From: Maxime Ripard @ 2016-12-15 12:17 UTC (permalink / raw)
To: Emmanuel Vadot
Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
wens-jdAy2FN1RRM, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161215123533.f7e2373d2a9eb113ee7b0600-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]
On Thu, Dec 15, 2016 at 12:35:33PM +0100, Emmanuel Vadot wrote:
>
> Hi Maxime,
>
> On Wed, 14 Dec 2016 16:30:13 +0100
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>
> > On Wed, Dec 14, 2016 at 03:57:24PM +0100, Emmanuel Vadot wrote:
> > > The node name for the power seq pin is mmc2@0 like the mmc2_pins_a one.
> > > This makes the original node (mmc2_pins_a) scrapped out of the dtb and
> > > result in a unusable eMMC if U-Boot didn't configured the pins to the
> > > correct functions.
> > >
> > > Changes since v1:
> > > * Rename the node mmc2-rst-pin
> >
> > That changelog should be after the ---. Removed it and applied.
> >
> > Thanks!
> > Maxime
>
> Sorry, still kinda new at doing patches for Linux, will be more
> carefull next time.
> Quick question, when you say applied, applied where exactly ? I had a
> quick look at your branches on git.kernel.org didn't find anything.
In general, my tree on kernel.org.
In this case, my local tree for now. We're right in the middle of the
merge window for 4.10, so in order not to pollute next and not to
confuse everyone (or rebasing a branch at some point), I just gather
the patches here. I'll publish a branch based on 4.10 as soon as
4.10-rc1 is released.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] ASoC: cs43130: Add support for CS43130 codec
From: Mark Brown @ 2016-12-15 12:12 UTC (permalink / raw)
To: Li Xu
Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
brian.austin-jGc1dHjMKG3QT0dZR+AlfA,
Paul.Handrigan-jGc1dHjMKG3QT0dZR+AlfA
In-Reply-To: <4c1d62fd-3bcf-4996-a942-bfcf6d7dce52-k7YZYYsDncjfk+Ne4bZl5AC/G2K4zDHf@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 19756 bytes --]
On Tue, Dec 13, 2016 at 11:47:06AM -0600, Li Xu wrote:
> + dev_dbg(codec->dev, "%s: cs43130->mclk = %d, cs43130->pll_out = %d",
> + __func__, cs43130->mclk, cs43130->pll_out);
If the standard style for logging output in the kernel were to include
the function name then dev_ would already do that. It isn't so they
don't - I think I also saw this affecting some of the non-debugging
messages.
> + if (cs43130->xtal_ibias > 0) {
> + usleep_range(1000, 1050);
> + /*PDN_XTAL = 0,enable*/
To repeat what I said last time:
| Please use the standard kernel coding style, need spaces here.
Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed. Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing. If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.
> + regmap_update_bits(cs43130->regmap,
> + CS43130_PWDN_CTL,
> + CS43130_PDN_XTAL_MASK,
> + 0 << CS43130_PDN_XTAL_SHIFT);
> + }
> +
> + /* PLL_START = 0, disable PLL_START */
> + regmap_update_bits(cs43130->regmap,
> + CS43130_PLL_SET_1,
> + CS43130_PLL_START_MASK,
> + 0 << CS43130_PLL_START_MASK);
The indentation style here isn't consistent even between the two
adjacent function calls :( . The first one is more the normal coding
style for the kernel, keeping each row of arguments aligned.
I'm picking up a lot of coding style stuff here - it really is
important, people read as much by pattern matching as by reading
individual letters so it does make it harder for people and poor coding
style also tends to be a very good indicator that there's other problems
in the code with poor understanding of APIs and so on.
> + bitwidth = (cs43130->dai_bit+1)*8;
Coding style, use spaces.
> + SOC_SINGLE("Swap L/R", CS43130_PCM_PATH_CTL_2, 1, 1, 0),
> + SOC_SINGLE("Copy L/R", CS43130_PCM_PATH_CTL_2, 0, 1, 0),
These should really be DAPM muxes.
> + /* ASP_3ST = 0 in master mode */
> + if (cs43130->dai_mode)
> + regmap_update_bits(cs43130->regmap, CS43130_PAD_INT_CFG,
> + 0x01, 0x00);
Again, why do we never need to undo this? If we never need to undo it
why not just do it unconditionally on probe?
> + SND_SOC_DAPM_DAC_E("HiFi DAC",
> + NULL, CS43130_PWDN_CTL, CS43130_PDN_HP_SHIFT, 1,
> + cs43130_dac_event,
> + (SND_SOC_DAPM_PRE_PMD)
> + ),
Like I said last time:
| There's no need for the ); to be on a new line here, nor for the extra
| indentation on the line before. There are lots more coding style
| issues, checkpatch will probably pick up many of them.
You also don't need the brackets around the DAPM constants.
> + regmap_read(cs43130->regmap, CS43130_PCM_PATH_CTL_1, ®);
> + mute_reg = reg & 0xfc;
> + if (mute)
> + regmap_write(cs43130->regmap, CS43130_PCM_PATH_CTL_1,
> + mute_reg | 0x03);
> + else
> + regmap_write(cs43130->regmap, CS43130_PCM_PATH_CTL_1, mute_reg);
This looks like you're open coding regmap_update_bits().
> + if (freq_in < 9600000 || freq_in > 26000000) {
> + dev_err(codec->dev,
> + "unsupported pll input reference clock:%d\n", freq_in);
> + return -EINVAL;
> + }
> +
> + switch (freq_in) {
> + case 9600000:
> + case 11289600:
> + case 12000000:
> + case 12288000:
> + case 13000000:
> + case 19200000:
> + case 22579200:
> + case 24000000:
> + case 24576000:
> + case 26000000:
> + cs43130->mclk = freq_in;
> + break;
> + default:
> + dev_err(codec->dev,
> + "unsupported pll input reference clock:%d\n", freq_in);
> + return -EINVAL;
> + }
You've got both a range check and a list of explicit supported
frequencies here - surely the range check is redundant?
> + /* Read all INT status and mask reg */
> + regmap_bulk_read(cs43130->regmap, CS43130_INT_STATUS_1,
> + stickies, CS43130_NUM_INT * sizeof(unsigned int));
> + regmap_bulk_read(cs43130->regmap, CS43130_INT_MASK_1,
> + masks, CS43130_NUM_INT * sizeof(unsigned int));
These are going to be buffer overflows, regmap bulk_read() takes a
number of registers to read not a number of bytes.
> + default:
> + dev_info(&i2c_client->dev,
> + "cirrus,xtal-ibias value or xtal unused %d",
> + val);
> + }
Missing break and this should be an error.
> + /* Enable interrupt handler */
> + ret = devm_request_threaded_irq(&client->dev,
> + client->irq,
> + NULL, cs43130_irq_thread,
> + IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> + "cs43130", cs43130);
> + if (ret != 0) {
> + dev_err(&client->dev, "Failed to request IRQ: %d\n", ret);
> + return ret;
> + }
> +
> + /* Unmask INT */
> + regmap_update_bits(cs43130->regmap, CS43130_INT_MASK_1,
> + CS43130_XTAL_RDY_INT | CS43130_XTAL_ERR_INT, 0);
We don't mask the interrupts when removing the driver. This means the
interrupt could fire as the device is being unbound which might crash
since the interrupt handler will still be registered as you're using
devm_ here. It's generally better not to use devm_ for interrupts, it's
hard to get right.
> + pm_runtime_disable(&client->dev);
We never enabled runtime PM.
> + regulator_bulk_disable(CS43130_NUM_SUPPLIES,
> + cs43130->supplies);
The indentation of the second line here is weird - normally it'd be
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int cs43130_runtime_suspend(struct device *dev)
> +{
> + struct cs43130_private *cs43130 = dev_get_drvdata(dev);
> +
> + regcache_cache_only(cs43130->regmap, true);
> + regcache_mark_dirty(cs43130->regmap);
> +
> + gpiod_set_value_cansleep(cs43130->reset_gpio, 0);
> +
> + regulator_bulk_disable(CS43130_NUM_SUPPLIES,
> + cs43130->supplies);
> + return 0;
> +}
> +
> +static int cs43130_runtime_resume(struct device *dev)
> +{
> + struct cs43130_private *cs43130 = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = regulator_bulk_enable(CS43130_NUM_SUPPLIES,
> + cs43130->supplies);
> + if (ret != 0) {
> + dev_err(dev, "Failed to enable supplies: %d\n",
> + ret);
> + return ret;
> + }
> +
> + regcache_cache_only(cs43130->regmap, false);
> +
> + gpiod_set_value_cansleep(cs43130->reset_gpio, 1);
> +
> + usleep_range(2000, 2050);
> +
> + ret = regcache_sync(cs43130->regmap);
> + if (ret != 0) {
> + dev_err(dev, "Failed to restore register cache\n");
> + goto err;
> + }
> + return 0;
> +err:
> + regcache_cache_only(cs43130->regmap, true);
> + regulator_bulk_disable(CS43130_NUM_SUPPLIES,
> + cs43130->supplies);
> +
> + return ret;
> +}
> +#endif
> +
> +static const struct dev_pm_ops cs43130_runtime_pm = {
> + SET_RUNTIME_PM_OPS(cs43130_runtime_suspend, cs43130_runtime_resume,
> + NULL)
> +};
> +
> +static const struct of_device_id cs43130_of_match[] = {
> + { .compatible = "cirrus,cs43130", },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, cs43130_of_match);
> +
> +static const struct i2c_device_id cs43130_i2c_id[] = {
> + {"cs43130", 0},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, cs43130_i2c_id);
> +
> +static struct i2c_driver cs43130_i2c_driver = {
> + .driver = {
> + .name = "cs43130",
> + .of_match_table = cs43130_of_match,
> + },
> + .id_table = cs43130_i2c_id,
> + .probe = cs43130_i2c_probe,
> + .remove = cs43130_i2c_remove,
> +};
> +
> +module_i2c_driver(cs43130_i2c_driver);
> +
> +MODULE_AUTHOR("Li Xu <li.xu-jGc1dHjMKG3QT0dZR+AlfA@public.gmane.org>");
> +MODULE_DESCRIPTION("Cirrus Logic CS43130 ALSA SoC Codec Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/codecs/cs43130.h b/sound/soc/codecs/cs43130.h
> new file mode 100644
> index 0000000..bceae76
> --- /dev/null
> +++ b/sound/soc/codecs/cs43130.h
> @@ -0,0 +1,268 @@
> +/*
> + * ALSA SoC CS43130 codec driver
> + *
> + * Copyright 2016 Cirrus Logic, Inc.
> + *
> + * Author: Li Xu <li.xu-jGc1dHjMKG3QT0dZR+AlfA@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program 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.
> + *
> + */
> +
> +#ifndef __CS43130_H__
> +#define __CS43130_H__
> +
> +/* CS43130 registers addresses */
> +/* all reg address is shifted by a byte for control byte to be LSB */
> +#define CS43130_FIRSTREG 0x010000
> +#define CS43130_LASTREG 0x0F0014
> +#define CS43130_CHIP_ID 0x00043130
> +#define CS4399_CHIP_ID 0x00043990
> +#define CS43130_DEVID_AB 0x010000 /*Device ID A & B [RO]*/
> +#define CS43130_DEVID_CD 0x010001 /*Device ID C & D [RO]*/
> +#define CS43130_DEVID_E 0x010002 /*Device ID E [RO]*/
> +#define CS43130_FAB_ID 0x010003 /*Fab ID [RO]*/
> +#define CS43130_REV_ID 0x010004 /*Revision ID [RO]*/
> +#define CS43130_SUBREV_ID 0x010005 /*Subrevision ID*/
> +#define CS43130_SYS_CLK_CTL_1 0x010006 /*System Clocking Ctl 1*/
> +#define CS43130_SP_SRATE 0x01000B /*Serial Port Sample Rate*/
> +#define CS43130_SP_BITSIZE 0x01000C /*Serial Port Bit Size*/
> +#define CS43130_PAD_INT_CFG 0x01000D /*Pad Interface Config*/
> +#define CS43130_DXD1 0x010010 /*DXD1*/
> +#define CS43130_PWDN_CTL 0x020000 /*Power Down Ctl*/
> +#define CS43130_DXD2 0x020019 /*DXD2*/
> +#define CS43130_CRYSTAL_SET 0x020052 /*Crystal Setting*/
> +#define CS43130_PLL_SET_1 0x030001 /*PLL Setting 1*/
> +#define CS43130_PLL_SET_2 0x030002 /*PLL Setting 2*/
> +#define CS43130_PLL_SET_3 0x030003 /*PLL Setting 3*/
> +#define CS43130_PLL_SET_4 0x030004 /*PLL Setting 4*/
> +#define CS43130_PLL_SET_5 0x030005 /*PLL Setting 5*/
> +#define CS43130_PLL_SET_6 0x030008 /*PLL Setting 6*/
> +#define CS43130_PLL_SET_7 0x03000A /*PLL Setting 7*/
> +#define CS43130_PLL_SET_8 0x03001B /*PLL Setting 8*/
> +#define CS43130_PLL_SET_9 0x040002 /*PLL Setting 9*/
> +#define CS43130_PLL_SET_10 0x040003 /*PLL Setting 10*/
> +#define CS43130_CLKOUT_CTL 0x040004 /*CLKOUT Ctl*/
> +#define CS43130_ASP_NUM_1 0x040010 /*ASP Numerator 1*/
> +#define CS43130_ASP_NUM_2 0x040011 /*ASP Numerator 2*/
> +#define CS43130_ASP_DENOM_1 0x040012 /*ASP Denominator 1*/
> +#define CS43130_ASP_DENOM_2 0x040013 /*ASP Denominator 2*/
> +#define CS43130_ASP_LRCK_HI_TIME_1 0x040014 /*ASP LRCK High Time 1*/
> +#define CS43130_ASP_LRCK_HI_TIME_2 0x040015 /*ASP LRCK High Time 2*/
> +#define CS43130_ASP_LRCK_PERIOD_1 0x040016 /*ASP LRCK Period 1*/
> +#define CS43130_ASP_LRCK_PERIOD_2 0x040017 /*ASP LRCK Period 2*/
> +#define CS43130_ASP_CLOCK_CONF 0x040018 /*ASP Clock Config*/
> +#define CS43130_ASP_FRAME_CONF 0x040019 /*ASP Frame Config*/
> +#define CS43130_XSP_NUM_1 0x040020 /*XSP Numerator 1*/
> +#define CS43130_XSP_NUM_2 0x040021 /*XSP Numerator 2*/
> +#define CS43130_XSP_DENOM_1 0x040022 /*XSP Denominator 1*/
> +#define CS43130_XSP_DENOM_2 0x040023 /*XSP Denominator 2*/
> +#define CS43130_XSP_LRCK_HI_TIME_1 0x040024 /*XSP LRCK High Time 1*/
> +#define CS43130_XSP_LRCK_HI_TIME_2 0x040025 /*XSP LRCK High Time 2*/
> +#define CS43130_XSP_LRCK_PERIOD_1 0x040026 /*XSP LRCK Period 1*/
> +#define CS43130_XSP_LRCK_PERIOD_2 0x040027 /*XSP LRCK Period 2*/
> +#define CS43130_XSP_CLOCK_CONF 0x040028 /*XSP Clock Config*/
> +#define CS43130_XSP_FRAME_CONF 0x040029 /*XSP Frame Config*/
> +#define CS43130_ASP_CH_1_LOC 0x050000 /*ASP Chan 1 Location*/
> +#define CS43130_ASP_CH_2_LOC 0x050001 /*ASP Chan 2 Location*/
> +#define CS43130_ASP_CH_1_SZ_EN 0x05000A /*ASP Chan 1 Size, Enable*/
> +#define CS43130_ASP_CH_2_SZ_EN 0x05000B /*ASP Chan 2 Size, Enable*/
> +#define CS43130_XSP_CH_1_LOC 0x060000 /*XSP Chan 1 Location*/
> +#define CS43130_XSP_CH_2_LOC 0x060001 /*XSP Chan 2 Location*/
> +#define CS43130_XSP_CH_1_SZ_EN 0x06000A /*XSP Chan 1 Size, Enable*/
> +#define CS43130_XSP_CH_2_SZ_EN 0x06000B /*XSP Chan 2 Size, Enable*/
> +#define CS43130_DSD_VOL_B 0x070000 /*DSD Volume B*/
> +#define CS43130_DSD_VOL_A 0x070001 /*DSD Volume A*/
> +#define CS43130_DSD_PATH_CTL_1 0x070002 /*DSD Proc Path Sig Ctl 1*/
> +#define CS43130_DSD_INT_CFG 0x070003 /*DSD Interface Config*/
> +#define CS43130_DSD_PATH_CTL_2 0x070004 /*DSD Proc Path Sig Ctl 2*/
> +#define CS43130_DSD_PCM_MIX_CTL 0x070005 /*DSD and PCM Mixing Ctl*/
> +#define CS43130_DSD_PATH_CTL_3 0x070006 /*DSD Proc Path Sig Ctl 3*/
> +#define CS43130_HP_OUT_CTL_1 0x080000 /*HP Output Ctl 1*/
> +#define CS43130_PCM_FILT_OPT 0x090000 /*PCM Filter Option*/
> +#define CS43130_PCM_VOL_B 0x090001 /*PCM Volume B*/
> +#define CS43130_PCM_VOL_A 0x090002 /*PCM Volume A*/
> +#define CS43130_PCM_PATH_CTL_1 0x090003 /*PCM Path Signal Ctl 1*/
> +#define CS43130_PCM_PATH_CTL_2 0x090004 /*PCM Path Signal Ctl 2*/
> +#define CS43130_CLASS_H_CTL 0x0B0000 /*Class H Ctl*/
> +#define CS43130_HP_DETECT 0x0D0000 /*HP Detect*/
> +#define CS43130_HP_STATUS 0x0D0001 /*HP Status [RO]*/
> +#define CS43130_HP_LOAD_1 0x0E0000 /*HP Load 1*/
> +#define CS43130_HP_MEAS_LOAD_1 0x0E0003 /*HP Load Measurement 1*/
> +#define CS43130_HP_MEAS_LOAD_2 0x0E0004 /*HP Load Measurement 2*/
> +#define CS43130_HP_DC_STAT_1 0x0E000D /*HP DC Load Status 0 [RO]*/
> +#define CS43130_HP_DC_STAT_2 0x0E000E /*HP DC Load Status 1 [RO]*/
> +#define CS43130_HP_AC_STAT_1 0x0E0010 /*HP AC Load Status 0 [RO]*/
> +#define CS43130_HP_AC_STAT_2 0x0E0011 /*HP AC Load Status 1 [RO]*/
> +#define CS43130_HP_LOAD_STAT 0x0E001A /*HP Load Status [RO]*/
> +#define CS43130_INT_STATUS_1 0x0F0000 /*Interrupt Status 1*/
> +#define CS43130_INT_STATUS_2 0x0F0001 /*Interrupt Status 2*/
> +#define CS43130_INT_STATUS_3 0x0F0002 /*Interrupt Status 3*/
> +#define CS43130_INT_STATUS_4 0x0F0003 /*Interrupt Status 4*/
> +#define CS43130_INT_STATUS_5 0x0F0004 /*Interrupt Status 5*/
> +#define CS43130_INT_MASK_1 0x0F0010 /*Interrupt Mask 1*/
> +#define CS43130_INT_MASK_2 0x0F0011 /*Interrupt Mask 2*/
> +#define CS43130_INT_MASK_3 0x0F0012 /*Interrupt Mask 3*/
> +#define CS43130_INT_MASK_4 0x0F0013 /*Interrupt Mask 4*/
> +#define CS43130_INT_MASK_5 0x0F0014 /*Interrupt Mask 5*/
> +
> +#define CS43130_MCLK_SRC_SEL_MASK 0x03
> +#define CS43130_MCLK_SRC_SEL_SHIFT 0
> +#define CS43130_MCLK_INT_MASK 0x04
> +#define CS43130_MCLK_INT_SHIFT 2
> +#define CS43130_SP_SRATE_MASK 0x0F
> +#define CS43130_SP_SRATE_SHIFT 0
> +#define CS43130_SP_BITSIZE_ASP_MASK 0x03
> +#define CS43130_SP_BITSIZE_ASP_SHIFT 0
> +#define CS43130_HP_DETECT_CTRL_SHIFT 6
> +#define CS43130_HP_DETECT_CTRL_MASK (0x03 << CS43130_HP_DETECT_CTRL_SHIFT)
> +#define CS43130_HP_DETECT_INV_SHIFT 5
> +#define CS43130_HP_DETECT_INV_MASK (1 << CS43130_HP_DETECT_INV_SHIFT)
> +
> +/* CS43130_INT_MASK_1 */
> +#define CS43130_HP_PLUG_INT_SHIFT 6
> +#define CS43130_HP_PLUG_INT (1 << CS43130_HP_PLUG_INT_SHIFT)
> +#define CS43130_HP_UNPLUG_INT_SHIFT 5
> +#define CS43130_HP_UNPLUG_INT (1 << CS43130_HP_UNPLUG_INT_SHIFT)
> +#define CS43130_XTAL_RDY_INT_SHIFT 4
> +#define CS43130_XTAL_RDY_INT (1 << CS43130_XTAL_RDY_INT_SHIFT)
> +#define CS43130_XTAL_ERR_INT_SHIFT 3
> +#define CS43130_XTAL_ERR_INT (1 << CS43130_XTAL_ERR_INT_SHIFT)
> +
> +/*Reg CS43130_SP_BITSIZE*/
> +#define CS43130_SP_BIT_SIZE_8 0x00
> +#define CS43130_SP_BIT_SIZE_16 0x01
> +#define CS43130_SP_BIT_SIZE_24 0x02
> +#define CS43130_SP_BIT_SIZE_32 0x03
> +
> +/*PLL*/
> +#define CS43130_PLL_START_MASK (0x1<<0)
> +#define CS43130_PLL_MODE_MASK 0x02
> +#define CS43130_PLL_MODE_SHIFT 1
> +
> +#define CS43130_PLL_REF_PREDIV_MASK 0x3
> +
> +#define CS43130_ASP_STP_MASK 0x10
> +#define CS43130_ASP_STP_SHIFT 4
> +#define CS43130_ASP_5050_MASK 0x08
> +#define CS43130_ASP_5050_SHIFT 3
> +#define CS43130_ASP_FSD_MASK 0x07
> +#define CS43130_ASP_FSD_SHIFT 0
> +
> +#define CS43130_ASP_MODE_MASK 0x10
> +#define CS43130_ASP_MODE_SHIFT 4
> +#define CS43130_ASP_SCPOL_OUT_MASK 0x08
> +#define CS43130_ASP_SCPOL_OUT_SHIFT 3
> +#define CS43130_ASP_SCPOL_IN_MASK 0x04
> +#define CS43130_ASP_SCPOL_IN_SHIFT 2
> +#define CS43130_ASP_LCPOL_OUT_MASK 0x02
> +#define CS43130_ASP_LCPOL_OUT_SHIFT 1
> +#define CS43130_ASP_LCPOL_IN_MASK 0x01
> +#define CS43130_ASP_LCPOL_IN_SHIFT 0
> +
> +/*Reg CS43130_PWDN_CTL*/
> +#define CS43130_PDN_XSP_MASK 0x80
> +#define CS43130_PDN_XSP_SHIFT 7
> +#define CS43130_PDN_ASP_MASK 0x40
> +#define CS43130_PDN_ASP_SHIFT 6
> +#define CS43130_PDN_DSPIF_MASK 0x20
> +#define CS43130_PDN_DSDIF_SHIFT 5
> +#define CS43130_PDN_HP_MASK 0x10
> +#define CS43130_PDN_HP_SHIFT 4
> +#define CS43130_PDN_XTAL_MASK 0x08
> +#define CS43130_PDN_XTAL_SHIFT 3
> +#define CS43130_PDN_PLL_MASK 0x04
> +#define CS43130_PDN_PLL_SHIFT 2
> +#define CS43130_PDN_CLKOUT_MASK 0x02
> +#define CS43130_PDN_CLKOUT_SHIFT 1
> +
> +#define CS43130_7_0_MASK 0xFF
> +#define CS43130_15_8_MASK 0xFF00
> +#define CS43130_23_16_MASK 0xFF0000
> +
> +/* Reg CS43130_HP_OUT_CTL_1 */
> +#define CS43130_HP_IN_EN_SHIFT 3
> +#define CS43130_HP_IN_EN_MASK 0x08
> +
> +#define CS43130_ASP_FORMATS (SNDRV_PCM_FMTBIT_S8 | \
> + SNDRV_PCM_FMTBIT_S16_LE | \
> + SNDRV_PCM_FMTBIT_S24_LE | \
> + SNDRV_PCM_FMTBIT_S32_LE)
> +
> +#define CS43130_XSP_FORMATS (SNDRV_PCM_FMTBIT_S24_LE | \
> + SNDRV_PCM_FMTBIT_S32_LE)
> +
> +enum cs43130_asp_rate {
> + CS43130_ASP_SPRATE_32K = 0,
> + CS43130_ASP_SPRATE_44_1K,
> + CS43130_ASP_SPRATE_48K,
> + CS43130_ASP_SPRATE_88_2K,
> + CS43130_ASP_SPRATE_96K,
> + CS43130_ASP_SPRATE_176_4K,
> + CS43130_ASP_SPRATE_192K,
> + CS43130_ASP_SPRATE_352_8K,
> + CS43130_ASP_SPRATE_384K,
> +};
> +
> +enum cs43130_mclk_src_sel {
> + CS43130_MCLK_SRC_XTAL = 0,
> + CS43130_MCLK_SRC_PLL,
> + CS43130_MCLK_SRC_RCO
> +};
> +
> +enum cs43130_mode {
> + CS43130_SLAVE_MODE = 0,
> + CS43130_MASTER_MODE
> +};
> +
> +enum cs43130_xtal_ibias {
> + CS43130_XTAL_IBIAS_15UA = 2,
> + CS43130_XTAL_IBIAS_12_5UA = 4,
> + CS43130_XTAL_IBIAS_7_5UA = 6,
> +};
> +
> +#define CS43130_AIF_BICK_RATE 1
> +#define CS43130_SYSCLK_MCLK 1
> +#define CS43130_NUM_SUPPLIES 5
> +static const char *const cs43130_supply_names[CS43130_NUM_SUPPLIES] = {
> + "VA",
> + "VP",
> + "VCP",
> + "VD",
> + "VL",
> +};
> +
> +#define CS43130_NUM_INT 5 /* number of interrupt status reg */
> +
> +struct cs43130_private {
> + struct snd_soc_codec *codec;
> + struct regmap *regmap;
> + struct regulator_bulk_data supplies[CS43130_NUM_SUPPLIES];
> + /* codec device ID */
> + unsigned int dev_id;
> + int mclk;
> + int sclk;
> + int xtal_ibias;
> +
> + bool pll_bypass;
> + int pll_out;
> + int mclk_int;
> + int dai_format;
> + int dai_mode;
> + int dai_bit;
> + int asp_size;
> + int fs;
> + bool bick_invert;
> + bool lrck_invert;
> + int bick;
> + struct gpio_desc *reset_gpio;
> +};
> +
> +#endif /* __CS43130_H__ */
> --
> 1.9.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v2] ARM: dts: Add missing CPU frequencies for Exynos5422/5800
From: Bartlomiej Zolnierkiewicz @ 2016-12-15 11:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Kukjin Kim, Javier Martinez Canillas, Rob Herring, Mark Rutland,
Russell King, Doug Anderson, Andreas Faerber, Thomas Abraham,
Ben Gamari, Arjun K V, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Add missing 2000MHz & 1900MHz OPPs (for A15 cores) and 1400MHz OPP
(for A7 cores). Also update common Odroid-XU3 Lite/XU3/XU4 thermal
cooling maps to account for new OPPs.
Since new OPPs are not available on all Exynos5422/5800 boards modify
dts files for Odroid-XU3 Lite (limited to 1.8 GHz / 1.3 GHz) & Peach
Pi (limited to 2.0 GHz / 1.3 GHz) accordingly.
Tested on Odroid-XU3 and XU3 Lite.
Cc: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Cc: Andreas Faerber <afaerber-l3A5Bk7waGM@public.gmane.org>
Cc: Thomas Abraham <thomas.ab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Ben Gamari <ben-OfdNVJEh3GKkIYD+M5K+dQ@public.gmane.org>
Cc: Arjun K V <arjun.kv-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
v2:
- added comments about limitations of SoC revisions used by Odroid-XU3 Lite and
Peach Pi boards (suggested by Javier)
- removed redundant opp_a7_14 label
- added Arjun to Cc:
Javier, could you test it on Peach Pi board?
arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 14 ++++++-------
arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts | 22 +++++++++++++++++++++
arch/arm/boot/dts/exynos5800-peach-pi.dts | 9 ++++++++
arch/arm/boot/dts/exynos5800.dtsi | 15 ++++++++++++++
4 files changed, 53 insertions(+), 7 deletions(-)
Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
===================================================================
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi 2016-12-15 12:43:54.365955950 +0100
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi 2016-12-15 12:43:54.361955949 +0100
@@ -118,7 +118,7 @@
/*
* When reaching cpu_alert3, reduce CPU
* by 2 steps. On Exynos5422/5800 that would
- * be: 1600 MHz and 1100 MHz.
+ * (usually) be: 1800 MHz and 1200 MHz.
*/
map3 {
trip = <&cpu_alert3>;
@@ -131,16 +131,16 @@
/*
* When reaching cpu_alert4, reduce CPU
- * further, down to 600 MHz (11 steps for big,
- * 7 steps for LITTLE).
+ * further, down to 600 MHz (13 steps for big,
+ * 8 steps for LITTLE).
*/
- map5 {
+ cooling_map5: map5 {
trip = <&cpu_alert4>;
- cooling-device = <&cpu0 3 7>;
+ cooling-device = <&cpu0 3 8>;
};
- map6 {
+ cooling_map6: map6 {
trip = <&cpu_alert4>;
- cooling-device = <&cpu4 3 11>;
+ cooling-device = <&cpu4 3 13>;
};
};
};
Index: b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
===================================================================
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts 2016-12-15 12:43:54.365955950 +0100
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts 2016-12-15 12:43:54.361955949 +0100
@@ -21,6 +21,28 @@
compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
};
+/*
+ * Odroid XU3-Lite board uses SoC revision with lower maximum frequencies
+ * than Odroid XU3/XU4 boards: 1.8 GHz for A15 cores & 1.3 GHz for A7 cores.
+ * Therefore we need to update OPPs tables and thermal maps accordingly.
+ */
+&cluster_a15_opp_table {
+ /delete-node/opp@2000000000;
+ /delete-node/opp@1900000000;
+};
+
+&cluster_a7_opp_table {
+ /delete-node/opp@1400000000;
+};
+
+&cooling_map5 {
+ cooling-device = <&cpu0 3 7>;
+};
+
+&cooling_map6 {
+ cooling-device = <&cpu4 3 11>;
+};
+
&pwm {
/*
* PWM 0 -- fan
Index: b/arch/arm/boot/dts/exynos5800-peach-pi.dts
===================================================================
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts 2016-12-15 12:43:54.365955950 +0100
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts 2016-12-15 12:43:54.361955949 +0100
@@ -146,6 +146,15 @@
vdd-supply = <&ldo9_reg>;
};
+/*
+ * Peach Pi board uses SoC revision with lower maximum frequency for A7 cores
+ * (1.3 GHz instead of 1.4 GHz) than Odroid XU3/XU4 boards. Thus we need to
+ * update A7 OPPs table accordingly.
+ */
+&cluster_a7_opp_table {
+ /delete-property/opp@1400000000;
+};
+
&cpu0 {
cpu-supply = <&buck2_reg>;
};
Index: b/arch/arm/boot/dts/exynos5800.dtsi
===================================================================
--- a/arch/arm/boot/dts/exynos5800.dtsi 2016-12-15 12:43:54.365955950 +0100
+++ b/arch/arm/boot/dts/exynos5800.dtsi 2016-12-15 12:43:54.361955949 +0100
@@ -24,6 +24,16 @@
};
&cluster_a15_opp_table {
+ opp@2000000000 {
+ opp-hz = /bits/ 64 <2000000000>;
+ opp-microvolt = <1250000>;
+ clock-latency-ns = <140000>;
+ };
+ opp@1900000000 {
+ opp-hz = /bits/ 64 <1900000000>;
+ opp-microvolt = <1250000>;
+ clock-latency-ns = <140000>;
+ };
opp@1700000000 {
opp-microvolt = <1250000>;
};
@@ -85,6 +95,11 @@
};
&cluster_a7_opp_table {
+ opp@1400000000 {
+ opp-hz = /bits/ 64 <1400000000>;
+ opp-microvolt = <1250000>;
+ clock-latency-ns = <140000>;
+ };
opp@1300000000 {
opp-microvolt = <1250000>;
};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: sunxi: Change node name for pwrseq pin on Olinuxino-lime2-emmc
From: Emmanuel Vadot @ 2016-12-15 11:35 UTC (permalink / raw)
To: Maxime Ripard
Cc: mark.rutland, devicetree, linux, linux-kernel, wens, robh+dt,
linux-arm-kernel
In-Reply-To: <20161214153013.bkdm3pu3osepfnn2@lukather>
Hi Maxime,
On Wed, 14 Dec 2016 16:30:13 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> On Wed, Dec 14, 2016 at 03:57:24PM +0100, Emmanuel Vadot wrote:
> > The node name for the power seq pin is mmc2@0 like the mmc2_pins_a one.
> > This makes the original node (mmc2_pins_a) scrapped out of the dtb and
> > result in a unusable eMMC if U-Boot didn't configured the pins to the
> > correct functions.
> >
> > Changes since v1:
> > * Rename the node mmc2-rst-pin
>
> That changelog should be after the ---. Removed it and applied.
>
> Thanks!
> Maxime
Sorry, still kinda new at doing patches for Linux, will be more
carefull next time.
Quick question, when you say applied, applied where exactly ? I had a
quick look at your branches on git.kernel.org didn't find anything.
Thanks.
--
Emmanuel Vadot <manu@bidouilliste.com> <manu@freebsd.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox