* Re: [PATCH 3/3] drm/tinydrm: new driver for ILI9341 display panels
From: Andy Shevchenko @ 2018-05-15 21:43 UTC (permalink / raw)
To: David Lechner
Cc: Mark Rutland, devicetree, limor, Linux Kernel Mailing List,
Rob Herring, dri-devel, Nitin Patil
In-Reply-To: <20180515014349.26226-4-david@lechnology.com>
On Tue, May 15, 2018 at 4:43 AM, David Lechner <david@lechnology.com> wrote:
> This adds a new driver for display panels that use the Ilitek ILI9341
> controller. It currently supports a single display panel, namely
> the YX240QV29-T (e.g. Adafruit 2.4" TFT).
>
> The init sequence is from the Adafruit Python library for the ILI9341
> controller. https://github.com/adafruit/Adafruit_Python_ILI9341
Some minor style nitpicks, otherwise LGTM
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
> MAINTAINERS | 6 +
> drivers/gpu/drm/tinydrm/Kconfig | 10 ++
> drivers/gpu/drm/tinydrm/Makefile | 1 +
> drivers/gpu/drm/tinydrm/ili9341.c | 239 ++++++++++++++++++++++++++++++
> 4 files changed, 256 insertions(+)
> create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bc219de9cbee..ffa099abbd79 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4480,6 +4480,12 @@ S: Maintained
> F: drivers/gpu/drm/tinydrm/ili9225.c
> F: Documentation/devicetree/bindings/display/ilitek,ili9225.txt
>
> +DRM DRIVER FOR ILITEK ILI9341 PANELS
> +M: David Lechner <david@lechnology.com>
> +S: Maintained
> +F: drivers/gpu/drm/tinydrm/ili9341.c
> +F: Documentation/devicetree/bindings/display/ilitek,ili9341.txt
> +
> DRM DRIVER FOR INTEL I810 VIDEO CARDS
> S: Orphan / Obsolete
> F: drivers/gpu/drm/i810/
> diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
> index 4592a5e3f20b..7a8008b0783f 100644
> --- a/drivers/gpu/drm/tinydrm/Kconfig
> +++ b/drivers/gpu/drm/tinydrm/Kconfig
> @@ -20,6 +20,16 @@ config TINYDRM_ILI9225
>
> If M is selected the module will be called ili9225.
>
> +config TINYDRM_ILI9341
> + tristate "DRM support for ILI9341 display panels"
> + depends on DRM_TINYDRM && SPI
Can't we do something like
if SPI
...
endif
?
> + select TINYDRM_MIPI_DBI
> + help
> + DRM driver for the following Ilitek ILI9341 panels:
> + * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
> +
> + If M is selected the module will be called ili9341.
> +
> config TINYDRM_MI0283QT
> tristate "DRM support for MI0283QT"
> depends on DRM_TINYDRM && SPI
> diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
> index 49a111929724..14d99080665a 100644
> --- a/drivers/gpu/drm/tinydrm/Makefile
> +++ b/drivers/gpu/drm/tinydrm/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI) += mipi-dbi.o
>
> # Displays
> obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o
> +obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o
> obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o
> obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o
> obj-$(CONFIG_TINYDRM_ST7586) += st7586.o
> diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
> new file mode 100644
> index 000000000000..2ce4244a68c3
> --- /dev/null
> +++ b/drivers/gpu/drm/tinydrm/ili9341.c
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * DRM driver for Ilitek ILI9341 panels
> + *
> + * Copyright 2018 David Lechner <david@lechnology.com>
> + *
> + * Based on mi0283qt.c:
> + * Copyright 2016 Noralf Trønnes
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_modeset_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
Can it be in order?
> +#include <drm/tinydrm/mipi-dbi.h>
> +#include <drm/tinydrm/tinydrm-helpers.h>
> +#include <video/mipi_display.h>
> +
> +#define ILI9341_FRMCTR1 0xb1
> +#define ILI9341_DISCTRL 0xb6
> +#define ILI9341_ETMOD 0xb7
> +
> +#define ILI9341_PWCTRL1 0xc0
> +#define ILI9341_PWCTRL2 0xc1
> +#define ILI9341_VMCTRL1 0xc5
> +#define ILI9341_VMCTRL2 0xc7
> +#define ILI9341_PWCTRLA 0xcb
> +#define ILI9341_PWCTRLB 0xcf
> +
> +#define ILI9341_PGAMCTRL 0xe0
> +#define ILI9341_NGAMCTRL 0xe1
> +#define ILI9341_DTCTRLA 0xe8
> +#define ILI9341_DTCTRLB 0xea
> +#define ILI9341_PWRSEQ 0xed
> +
> +#define ILI9341_EN3GAM 0xf2
> +#define ILI9341_PUMPCTRL 0xf7
> +
> +#define ILI9341_MADCTL_BGR BIT(3)
> +#define ILI9341_MADCTL_MV BIT(5)
> +#define ILI9341_MADCTL_MX BIT(6)
> +#define ILI9341_MADCTL_MY BIT(7)
> +
> +static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
> + struct drm_crtc_state *crtc_state,
> + struct drm_plane_state *plane_state)
> +{
> + struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
> + struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
> + u8 addr_mode;
> + int ret;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + ret = mipi_dbi_poweron_conditional_reset(mipi);
> + if (ret < 0)
> + return;
> + if (ret == 1)
> + goto out_enable;
> +
> + mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
> +
> + mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
> + mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
> + mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
> + mipi_dbi_command(mipi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
> + mipi_dbi_command(mipi, ILI9341_PUMPCTRL, 0x20);
> + mipi_dbi_command(mipi, ILI9341_DTCTRLB, 0x00, 0x00);
> +
> + /* Power Control */
> + mipi_dbi_command(mipi, ILI9341_PWCTRL1, 0x23);
> + mipi_dbi_command(mipi, ILI9341_PWCTRL2, 0x10);
> + /* VCOM */
> + mipi_dbi_command(mipi, ILI9341_VMCTRL1, 0x3e, 0x28);
> + mipi_dbi_command(mipi, ILI9341_VMCTRL2, 0x86);
> +
> + /* Memory Access Control */
> + mipi_dbi_command(mipi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT);
> +
> + /* Frame Rate */
> + mipi_dbi_command(mipi, ILI9341_FRMCTR1, 0x00, 0x1b);
> +
> + /* Gamma */
> + mipi_dbi_command(mipi, ILI9341_EN3GAM, 0x00);
> + mipi_dbi_command(mipi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
> + mipi_dbi_command(mipi, ILI9341_PGAMCTRL,
> + 0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
> + 0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
> + mipi_dbi_command(mipi, ILI9341_NGAMCTRL,
> + 0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
> + 0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
> +
> + /* DDRAM */
> + mipi_dbi_command(mipi, ILI9341_ETMOD, 0x07);
> +
> + /* Display */
> + mipi_dbi_command(mipi, ILI9341_DISCTRL, 0x08, 0x82, 0x27, 0x00);
> + mipi_dbi_command(mipi, MIPI_DCS_EXIT_SLEEP_MODE);
> + msleep(100);
> +
> + mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
> + msleep(100);
> +
> +out_enable:
> + switch (mipi->rotation) {
> + default:
> + addr_mode = ILI9341_MADCTL_MX;
> + break;
> + case 90:
> + addr_mode = ILI9341_MADCTL_MV;
> + break;
> + case 180:
> + addr_mode = ILI9341_MADCTL_MY;
> + break;
> + case 270:
> + addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
> + ILI9341_MADCTL_MX;
> + break;
> + }
> + addr_mode |= ILI9341_MADCTL_BGR;
> + mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
> + mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
> +}
> +
> +static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
> + .enable = yx240qv29_enable,
> + .disable = mipi_dbi_pipe_disable,
> + .update = tinydrm_display_pipe_update,
> + .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +};
> +
> +static const struct drm_display_mode yx240qv29_mode = {
> + TINYDRM_MODE(240, 320, 37, 49),
> +};
> +
> +DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
> +
> +static struct drm_driver ili9341_driver = {
> + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
> + DRIVER_ATOMIC,
I would rather keep one per line, or all at one line disregard 80
character limit.
> + .fops = &ili9341_fops,
> + TINYDRM_GEM_DRIVER_OPS,
> + .lastclose = drm_fb_helper_lastclose,
> + .debugfs_init = mipi_dbi_debugfs_init,
> + .name = "ili9341",
> + .desc = "Ilitek ILI9341",
> + .date = "20180514",
> + .major = 1,
> + .minor = 0,
> +};
> +
> +static const struct of_device_id ili9341_of_match[] = {
> + { .compatible = "noname,yx240qv29" },
> + {},
Terminators better w/o comma.
> +};
> +MODULE_DEVICE_TABLE(of, ili9341_of_match);
> +
> +static const struct spi_device_id ili9341_id[] = {
> + { "yx240qv29", 0 },
> + { },
Ditto.
> +};
> +MODULE_DEVICE_TABLE(spi, ili9341_id);
> +
> +static int ili9341_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct mipi_dbi *mipi;
> + struct gpio_desc *dc;
> + u32 rotation = 0;
> + int ret;
> +
> + mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> + if (!mipi)
> + return -ENOMEM;
> +
> + mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(mipi->reset)) {
> + DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
> + return PTR_ERR(mipi->reset);
> + }
> +
> + dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
> + if (IS_ERR(dc)) {
> + DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
> + return PTR_ERR(dc);
> + }
> +
> + mipi->regulator = devm_regulator_get(dev, "power");
> + if (IS_ERR(mipi->regulator))
> + return PTR_ERR(mipi->regulator);
> +
> + mipi->backlight = devm_of_find_backlight(dev);
> + if (IS_ERR(mipi->backlight))
> + return PTR_ERR(mipi->backlight);
> +
> + device_property_read_u32(dev, "rotation", &rotation);
> +
> + ret = mipi_dbi_spi_init(spi, mipi, dc);
> + if (ret)
> + return ret;
> +
> + ret = mipi_dbi_init(&spi->dev, mipi, &ili9341_pipe_funcs,
> + &ili9341_driver, &yx240qv29_mode, rotation);
> + if (ret)
> + return ret;
> +
> + spi_set_drvdata(spi, mipi);
> +
> + return devm_tinydrm_register(&mipi->tinydrm);
> +}
> +
> +static void ili9341_shutdown(struct spi_device *spi)
> +{
> + struct mipi_dbi *mipi = spi_get_drvdata(spi);
> +
> + tinydrm_shutdown(&mipi->tinydrm);
> +}
> +
> +static struct spi_driver ili9341_spi_driver = {
> + .driver = {
> + .name = "ili9341",
> + .of_match_table = ili9341_of_match,
> + },
> + .id_table = ili9341_id,
> + .probe = ili9341_probe,
> + .shutdown = ili9341_shutdown,
> +};
> +module_spi_driver(ili9341_spi_driver);
> +
> +MODULE_DESCRIPTION("Ilitek ILI9341 DRM driver");
> +MODULE_AUTHOR("David Lechner <david@lechnology.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.17.0
>
--
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v6 1/2] dt: bindings: lm3601x: Introduce the lm3601x driver
From: Dan Murphy @ 2018-05-15 21:29 UTC (permalink / raw)
To: Jacek Anaszewski, robh+dt, mark.rutland, pavel
Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <98319f30-2abe-158c-1b51-33d1df80e185@gmail.com>
Jacek
On 05/15/2018 04:13 PM, Jacek Anaszewski wrote:
> Hi Dan,
>
> Thanks for the update.
>
> On 05/15/2018 05:43 PM, Dan Murphy wrote:
>> Introduce the device tree bindings for the lm3601x
>> family of LED torch, flash and IR drivers.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>
>> v6 - Removed multiple led child nodes, fixed example to display micro ranges
>> for corresponding child nodes and added led-sources to define the current driver -
>> https://patchwork.kernel.org/patch/10392121/
>>
>> v5 - No changes - https://patchwork.kernel.org/patch/10391743/
>> v4 - Added " " around "=", changed strobe to flash on label, removed "support and
>> register" comment and change ir lable to ir:torch - See v2 patchworks for comments
>> v3 - Removed wildcard compatible - https://patchwork.kernel.org/patch/10386241/
>> v2 - No changes - https://patchwork.kernel.org/patch/10384587/
>>
>> .../devicetree/bindings/leds/leds-lm3601x.txt | 47 +++++++++++++++++++
>> 1 file changed, 47 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3601x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3601x.txt b/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
>> new file mode 100644
>> index 000000000000..27930a89e9a5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
>> @@ -0,0 +1,47 @@
>> +* Texas Instruments - lm3601x Single-LED Flash Driver
>> +
>> +The LM3601X are ultra-small LED flash drivers that
>> +provide a high level of adjustability.
>> +
>> +Required properties:
>> + - compatible : Can be one of the following
>> + "ti,lm36010"
>> + "ti,lm36011"
>> + - reg : I2C slave address
>> + - #address-cells : 1
>> + - #size-cells : 0
>> +
>> +Required child properties:
>> + - reg : 0
>> + - led-sources: 0 - Indicates a IR mode
>> + 1 - Indicates a Torch (white LED) mode
>
> You don't need led-sources at all. Please use reg as in
> the previous version. led-sources is useful for describing
> more complex arrangements. Here reg will do.
OK. Thought we would keep consistent with the max IC.
>
>> +
>> +Required properties for flash LED child nodes:
>> + See Documentation/devicetree/bindings/leds/common.txt
>> + - flash-max-microamp : Range from 11mA -> 1.5A
>> + - flash-max-timeout-us : Range from 40ms -> 1600ms
>> + - led-max-microamp : Range from 2.4mA -> 376mA
>
> Please give also a step in the format like below
> (taken from max77693):
>
> Valid values: 62500 - 1000000, step by 62500 (rounded down)
I would do this but the step is not linear. The step is 40ms from 40 -> 400.
after that the step goes to 200ms from 400->1600.
same with the current ranges.
Dan
>
>
>> +
>> +Optional child properties:
>> + - label : see Documentation/devicetree/bindings/leds/common.txt
>> +
>> +Example:
>> +led-controller@64 {
>> + compatible = "ti,lm36010";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0x64>;
>> +
>> + led@0 {
>> + reg = <0>;
>> + label = "white:torch";
>> + led-max-microamp = <376000>;
>> + flash-max-microamp = <1500000>;
>> + flash-max-timeout-us = <1600000>;
>> + led-sources = <1>;
>> + };
>> +}
>> +
>> +For more product information please see the links below:
>> +http://www.ti.com/product/LM36010
>> +http://www.ti.com/product/LM36011
>>
>
--
------------------
Dan Murphy
^ permalink raw reply
* Re: [PATCH v6 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Jacek Anaszewski @ 2018-05-15 21:24 UTC (permalink / raw)
To: Dan Murphy, robh+dt, mark.rutland, pavel
Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <20180515154352.20263-2-dmurphy@ti.com>
Dan,
Thanks for the update. Please refer to my comments below.
On 05/15/2018 05:43 PM, Dan Murphy wrote:
> Introduce the family of LED devices that can
> drive a torch, strobe or IR LED.
>
> The LED driver can be configured with a strobe
> timer to execute a strobe flash. The IR LED
> brightness is controlled via the torch brightness
> register.
>
> The data sheet for each the LM36010 and LM36011
> LED drivers can be found here:
> http://www.ti.com/product/LM36010
> http://www.ti.com/product/LM36011
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>
> v6 - This driver has been heavily modified from v5. There is no longer reading
> of individual child nodes. There are too many changes to list here see -
> https://patchwork.kernel.org/patch/10392123/
>
> v5 - Fixed magic numbers, change reg cache type, added of_put_node to release
> the dt node ref, and I did not change the remove function to leave the LED in its
> state on driver removal - https://patchwork.kernel.org/patch/10391741/
> v4 - Fixed Cocci issue using ARRAY_SIZE - https://patchwork.kernel.org/patch/10389259/
> v3 - removed wildcard dt compatible, fixed copyright, fixed struct doc, removed
> RO registers from default, added regmap volatile for FLAGS_REG, updated regmap cache type,
> fixed unlock and extra semi colon in strobe_set, removed unnecessary out label
> in led register and fixed checking of the ret in brightness_set - https://patchwork.kernel.org/patch/10386243/
> v2 - Fixed kbuild issue and removed unused cdev_strobe - https://patchwork.kernel.org/patch/10384585/
>
> drivers/leds/Kconfig | 9 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-lm3601x.c | 595 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 605 insertions(+)
> create mode 100644 drivers/leds/leds-lm3601x.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 2c896c0e69e1..50ae536f343f 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -145,6 +145,15 @@ config LEDS_LM3692X
> This option enables support for the TI LM3692x family
> of white LED string drivers used for backlighting.
>
> +config LEDS_LM3601X
> + tristate "LED support for LM3601x Chips"
> + depends on LEDS_CLASS && I2C && OF
> + depends on LEDS_CLASS_FLASH
> + select REGMAP_I2C
> + help
> + This option enables support for the TI LM3601x family
> + of flash, torch and indicator classes.
> +
> config LEDS_LOCOMO
> tristate "LED Support for Locomo device"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 91eca81cae82..b79807fe1b67 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
> obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o
> obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
> obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o
> +obj-$(CONFIG_LEDS_LM3601X) += leds-lm3601x.o
>
> # LED SPI Drivers
> obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
> diff --git a/drivers/leds/leds-lm3601x.c b/drivers/leds/leds-lm3601x.c
> new file mode 100644
> index 000000000000..fa87da5d5159
> --- /dev/null
> +++ b/drivers/leds/leds-lm3601x.c
> @@ -0,0 +1,595 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Flash and torch driver for Texas Instruments LM3601X LED
> +// Flash driver chip family
> +// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/leds.h>
> +#include <linux/led-class-flash.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <uapi/linux/uleds.h>
> +
> +#define LM3601X_LED_TORCH 0x0
> +#define LM3601X_LED_IR 0x1
> +
> +/* Registers */
> +#define LM3601X_ENABLE_REG 0x01
> +#define LM3601X_CFG_REG 0x02
> +#define LM3601X_LED_FLASH_REG 0x03
> +#define LM3601X_LED_TORCH_REG 0x04
> +#define LM3601X_FLAGS_REG 0x05
> +#define LM3601X_DEV_ID_REG 0x06
> +
> +#define LM3601X_SW_RESET BIT(7)
> +
> +/* Enable Mode bits */
> +#define LM3601X_MODE_STANDBY 0x00
> +#define LM3601X_MODE_IR_DRV BIT(0)
> +#define LM3601X_MODE_TORCH BIT(1)
> +#define LM3601X_MODE_STROBE (BIT(0) | BIT(1))
> +#define LM3601X_STRB_EN BIT(2)
> +#define LM3601X_STRB_LVL_TRIG ~BIT(3)
> +#define LM3601X_STRB_EDGE_TRIG BIT(3)
> +#define LM3601X_IVFM_EN BIT(4)
> +
> +#define LM36010_BOOST_LIMIT_19 ~BIT(5)
> +#define LM36010_BOOST_LIMIT_28 BIT(5)
> +#define LM36010_BOOST_FREQ_2MHZ ~BIT(6)
> +#define LM36010_BOOST_FREQ_4MHZ BIT(6)
> +#define LM36010_BOOST_MODE_NORM ~BIT(7)
> +#define LM36010_BOOST_MODE_PASS BIT(7)
> +
> +/* Flag Mask */
> +#define LM3601X_FLASH_TIME_OUT BIT(0)
> +#define LM3601X_UVLO_FAULT BIT(1)
> +#define LM3601X_THERM_SHUTDOWN BIT(2)
> +#define LM3601X_THERM_CURR BIT(3)
> +#define LM36010_CURR_LIMIT BIT(4)
> +#define LM3601X_SHORT_FAULT BIT(5)
> +#define LM3601X_IVFM_TRIP BIT(6)
> +#define LM36010_OVP_FAULT BIT(7)
> +
> +#define LM3601X_MIN_TORCH_I_UA 2400
> +#define LM3601X_MIN_STROBE_I_MA 11
> +
> +#define LM3601X_TIMEOUT_MASK 0x1e
> +#define LM3601X_ENABLE_MASK 0x03
> +
> +enum lm3601x_type {
> + CHIP_LM36010 = 0,
> + CHIP_LM36011,
> +};
> +
> +/**
> + * struct lm3601x_max_timeouts -
> + * @timeout: timeout value in ms
> + * @regval: the value of the register to write
> + */
> +struct lm3601x_max_timeouts {
> + int timeout;
> + int reg_val;
> +};
> +
> +/**
> + * struct lm3601x_led -
> + * @lock: Lock for reading/writing the device
> + * @regmap: Devices register map
> + * @client: Pointer to the I2C client
> + * @led_node: DT device node for the led
> + * @cdev_torch: led class device pointer for the torch
> + * @cdev_ir: led class device pointer for infrared
> + * @fled_cdev: flash led class device pointer
> + * @led_name: LED label for the Torch or IR LED
> + * @strobe: LED label for the strobe
> + * @last_flag: last known flags register value
> + * @strobe_timeout: the timeout for the strobe
> + * @torch_current_max: maximum current for the torch
> + * @strobe_current_max: maximum current for the strobe
> + * @max_strobe_timeout: maximum timeout for the strobe
> + * @led_mode: The mode to enable either IR or Torch
> + */
> +struct lm3601x_led {
> + struct mutex lock;
> + struct regmap *regmap;
> + struct i2c_client *client;
> +
> + struct device_node *led_node;
> +
> + struct led_classdev cdev_torch;
> + struct led_classdev cdev_ir;
> +
> + struct led_classdev_flash fled_cdev;
> +
> + char led_name[LED_MAX_NAME_SIZE];
> + char strobe[LED_MAX_NAME_SIZE];
> +
> + unsigned int last_flag;
> + unsigned int strobe_timeout;
> +
> + u32 torch_current_max;
> + u32 strobe_current_max;
> + u32 max_strobe_timeout;
> +
> + int led_mode;
> +};
> +
> +static const struct lm3601x_max_timeouts strobe_timeouts[] = {
> + { 40000, 0x00 },
> + { 80000, 0x01 },
> + { 120000, 0x02 },
> + { 160000, 0x03 },
> + { 200000, 0x04 },
> + { 240000, 0x05 },
> + { 280000, 0x06 },
> + { 320000, 0x07 },
> + { 360000, 0x08 },
> + { 400000, 0x09 },
> + { 600000, 0x0a },
> + { 800000, 0x0b },
> + { 1000000, 0x0c },
> + { 1200000, 0x0d },
> + { 1400000, 0x0e },
> + { 1600000, 0x0f },
> +};
> +
> +static const struct reg_default lm3601x_regmap_defs[] = {
> + { LM3601X_ENABLE_REG, 0x20 },
> + { LM3601X_CFG_REG, 0x15 },
> + { LM3601X_LED_FLASH_REG, 0x00 },
> + { LM3601X_LED_TORCH_REG, 0x00 },
> +};
> +
> +static bool lm3601x_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case LM3601X_FLAGS_REG:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct regmap_config lm3601x_regmap = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +
> + .max_register = LM3601X_DEV_ID_REG,
> + .reg_defaults = lm3601x_regmap_defs,
> + .num_reg_defaults = ARRAY_SIZE(lm3601x_regmap_defs),
> + .cache_type = REGCACHE_RBTREE,
> + .volatile_reg = lm3601x_volatile_reg,
> +};
> +
> +static struct lm3601x_led *fled_cdev_to_led(
> + struct led_classdev_flash *fled_cdev)
> +{
> + return container_of(fled_cdev, struct lm3601x_led, fled_cdev);
> +}
> +
> +static int lm3601x_read_faults(struct lm3601x_led *led)
> +{
> + int flags_val;
> + int ret;
> +
> + ret = regmap_read(led->regmap, LM3601X_FLAGS_REG, &flags_val);
> + if (ret < 0)
> + return -EIO;
> +
> + led->last_flag = 0;
> +
> + if (flags_val & LM36010_OVP_FAULT)
> + led->last_flag |= LED_FAULT_OVER_VOLTAGE;
> +
> + if (flags_val & (LM3601X_THERM_SHUTDOWN | LM3601X_THERM_CURR))
> + led->last_flag |= LED_FAULT_OVER_TEMPERATURE;
> +
> + if (flags_val & LM3601X_SHORT_FAULT)
> + led->last_flag |= LED_FAULT_SHORT_CIRCUIT;
> +
> + if (flags_val & LM36010_CURR_LIMIT)
> + led->last_flag |= LED_FAULT_OVER_CURRENT;
> +
> + if (flags_val & LM3601X_UVLO_FAULT)
> + led->last_flag |= LED_FAULT_UNDER_VOLTAGE;
> +
> + if (flags_val & LM3601X_IVFM_TRIP)
> + led->last_flag |= LED_FAULT_INPUT_VOLTAGE;
> +
> + if (flags_val & LM3601X_THERM_SHUTDOWN)
> + led->last_flag |= LED_FAULT_LED_OVER_TEMPERATURE;
> +
> + return led->last_flag;
> +}
> +
> +static int lm3601x_brightness_set(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct lm3601x_led *led =
> + container_of(cdev, struct lm3601x_led, cdev_torch);
> + u8 brightness_val;
> + int ret, led_mode_val;
> +
> + mutex_lock(&led->lock);
> +
> + ret = lm3601x_read_faults(led);
> + if (ret < 0)
> + goto out;
> +
> + if (led->led_mode == LM3601X_LED_TORCH)
> + led_mode_val = LM3601X_MODE_TORCH;
> + else
> + led_mode_val = LM3601X_MODE_IR_DRV;
> +
> + if (brightness == LED_OFF) {
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + led_mode_val, LED_OFF);
> + goto out;
> + }
> +
> + if (brightness == LED_ON)
> + brightness_val = LED_ON;
> + else
> + brightness_val = (brightness/2);
> +
> + ret = regmap_write(led->regmap, LM3601X_LED_TORCH_REG, brightness_val);
> + if (ret < 0)
> + goto out;
> +
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + led_mode_val,
> + led_mode_val);
> +
> +out:
> + mutex_unlock(&led->lock);
> + return ret;
> +}
> +
> +static int lm3601x_strobe_set(struct led_classdev_flash *fled_cdev,
> + bool state)
> +{
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> + int ret;
> + int current_timeout;
> + int reg_count;
> + int i;
> + int timeout_reg_val = 0;
> +
> + mutex_lock(&led->lock);
> +
> + ret = regmap_read(led->regmap, LM3601X_CFG_REG, ¤t_timeout);
> + if (ret < 0)
> + goto out;
> +
> + reg_count = ARRAY_SIZE(strobe_timeouts);
> + for (i = 0; i < reg_count; i++) {
> + if (led->strobe_timeout > strobe_timeouts[i].timeout)
> + continue;
> +
> + if (led->strobe_timeout <= strobe_timeouts[i].timeout) {
> + timeout_reg_val = (strobe_timeouts[i].reg_val << 1);
> + break;
> + }
> +
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (led->strobe_timeout != current_timeout)
> + ret = regmap_update_bits(led->regmap, LM3601X_CFG_REG,
> + LM3601X_TIMEOUT_MASK, timeout_reg_val);
> +
> + if (state)
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + LM3601X_MODE_STROBE,
> + LM3601X_MODE_STROBE);
> + else
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + LM3601X_MODE_STROBE, LED_OFF);
> +
> + ret = lm3601x_read_faults(led);
> +out:
> + mutex_unlock(&led->lock);
> + return ret;
> +}
> +
> +static int lm3601x_strobe_brightness_set(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(cdev);
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> + int ret;
> + u8 brightness_val;
> +
> + mutex_lock(&led->lock);
> + ret = lm3601x_read_faults(led);
> + if (ret < 0)
> + goto out;
> +
> + if (brightness == LED_OFF) {
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + LM3601X_MODE_STROBE, LED_OFF);
> + goto out;
> + }
> +
> + if (brightness == LED_ON)
> + brightness_val = LED_ON;
> + else
> + brightness_val = (brightness/2);
> +
> + ret = regmap_write(led->regmap, LM3601X_LED_FLASH_REG, brightness_val);
> +
> +out:
> + mutex_unlock(&led->lock);
> + return ret;
> +}
> +
> +static int lm3601x_strobe_timeout_set(struct led_classdev_flash *fled_cdev,
> + u32 timeout)
> +{
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> + int ret = 0;
> +
> + mutex_lock(&led->lock);
> +
> + led->strobe_timeout = timeout;
> +
> + mutex_unlock(&led->lock);
> +
> + return ret;
> +}
> +
> +static int lm3601x_strobe_get(struct led_classdev_flash *fled_cdev,
> + bool *state)
> +{
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> + int ret;
> + int strobe_state;
> +
> + mutex_lock(&led->lock);
> +
> + ret = regmap_read(led->regmap, LM3601X_ENABLE_REG, &strobe_state);
> + if (ret < 0)
> + goto out;
> +
> + *state = strobe_state & LM3601X_MODE_STROBE;
> +
> +out:
> + mutex_unlock(&led->lock);
> + return ret;
> +}
> +
> +static int lm3601x_strobe_fault_get(struct led_classdev_flash *fled_cdev,
> + u32 *fault)
> +{
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> +
> + lm3601x_read_faults(led);
> +
> + *fault = led->last_flag;
> +
> + return 0;
> +}
> +
> +static const struct led_flash_ops strobe_ops = {
> + .strobe_set = lm3601x_strobe_set,
> + .strobe_get = lm3601x_strobe_get,
> + .timeout_set = lm3601x_strobe_timeout_set,
> + .fault_get = lm3601x_strobe_fault_get,
You're still missing flash_brightness_set here.
> +};
> +
> +static int lm3601x_register_leds(struct lm3601x_led *led)
> +{
> + struct led_classdev_flash *fled_cdev;
> + struct led_classdev *led_cdev;
> + int err = -ENODEV;
> +
> + led->cdev_torch.name = led->led_name;
> + led->cdev_torch.max_brightness = LED_FULL;
This should be led->torch_current_max converted to brightness levels.
Please compare how leds-max77693 and leds-as3645a do that.
> + led->cdev_torch.brightness_set_blocking = lm3601x_brightness_set;
> + err = devm_led_classdev_register(&led->client->dev,
> + &led->cdev_torch);
> + if (err < 0)
> + return err;
You shouldn't register two LED class devices for a single DT child
node, and honestly I don't know why you would have to do it.
> +
> + fled_cdev = &led->fled_cdev;
> + fled_cdev->ops = &strobe_ops;
> +
> + led_cdev = &fled_cdev->led_cdev;
> + led_cdev->name = led->strobe;
> + led_cdev->max_brightness = LED_FUL
> + led_cdev->brightness_set_blocking = lm3601x_strobe_brightness_set;
> + led_cdev->flags |= LED_DEV_CAP_FLASH;
> +
> + err = led_classdev_flash_register(&led->client->dev,
> + fled_cdev);
Please look into the implementation of led_classdev_flash_register()
and compare other LED flash class drivers. What prevents you from
applying the same approach?
To put it concisely: LED flash class wrapper was designed to allow
for operating on a single LED in two modes: torch and flash.
Torch brightness is governed by brightness file and flash brightness
is governed by flash_brightness file. The difference between the two
modes is that torch mode is activated by writing value greater than 0
to the brightness file and flash mode is activated by writing 1 to
the strobe_set file.
In other words flash_brightness overwrites the (torch) brightness
for the time equal to the flash_timeout or until 0 is written
to the strobe_set file.
> + return err;
> +}
> +
> +static void lm3601x_init_flash_timeout(struct lm3601x_led *led)
> +{
> + struct led_flash_setting *setting;
> +
> + setting = &led->fled_cdev.timeout;
> + setting->min = strobe_timeouts[0].timeout;
> + setting->max = led->max_strobe_timeout;
> + setting->step = 40;
> + setting->val = led->max_strobe_timeout;
> +}
> +
> +static int lm3601x_parse_node(struct lm3601x_led *led,
> + struct device_node *node)
> +{
> + struct device_node *child_node;
> + const char *name;
> + char *mode_name;
> + int ret = -ENODEV;
> +
> + for_each_available_child_of_node(node, child_node) {
> + led->led_node = of_node_get(child_node);
> + if (!led->led_node) {
> + dev_err(&led->client->dev,
> + "No LED Child node\n");
> +
> + goto out_err;
> + }
> +
> + ret = of_property_read_u32(led->led_node, "led-sources",
> + &led->led_mode);
> + if (ret) {
> + dev_err(&led->client->dev,
> + "led-sources DT property missing\n");
> + goto out_err;
> + }
> +
> + if (led->led_mode < LM3601X_LED_TORCH ||
> + led->led_mode > LM3601X_LED_IR) {
> + dev_warn(&led->client->dev,
> + "Invalid led mode requested\n");
> +
> + goto out_err;
> +
> + }
> + }
> +
> + if (led->led_mode == LM3601X_LED_TORCH) {
> + ret = of_property_read_string(led->led_node, "label", &name);
> + if (!ret)
> + snprintf(led->led_name, sizeof(led->led_name),
> + "%s:%s", led->led_node->name, name);
First segment of a LED class device name should be "devicename".
> + else
> + snprintf(led->led_name, sizeof(led->led_name),
> + "%s:torch", led->led_node->name);
> +
> + ret = of_property_read_u32(led->led_node, "led-max-microamp",
> + &led->torch_current_max);
> + if (ret < 0) {
> + dev_warn(&led->client->dev,
> + "led-max-microamp DT property missing\n");
> +
> + goto out_err;
> + }
> +
> + mode_name = "torch";
> +
> + } else if (led->led_mode == LM3601X_LED_IR) {
> + ret = of_property_read_string(led->led_node, "label", &name);
> + if (!ret)
> + snprintf(led->led_name, sizeof(led->led_name),
> + "%s:%s", led->led_node->name, name);
> + else
> + snprintf(led->led_name, sizeof(led->led_name),
> + "%s::infrared", led->led_node->name);
Ditto.
> +
> + mode_name = "ir";
> +
> + } else {
> + dev_warn(&led->client->dev,
> + "No LED mode is selected exiting probe\n");
> +
> + goto out_err;
> + }
> +
> + /* Flash mode is available in IR or Torch mode so read the DT */
> + snprintf(led->strobe, sizeof(led->strobe),
> + "%s:%s:strobe", led->led_node->name, mode_name);
strobe is not a proper LED function name (it is not a noun).
Please use "flash" instead.
> +
> + ret = of_property_read_u32(led->led_node,
> + "flash-max-microamp",
> + &led->strobe_current_max);
> + if (ret < 0) {
> + dev_warn(&led->client->dev,
> + "flash-max-microamp DT property missing\n");
> + goto out_err;
> + }
> +
> + ret = of_property_read_u32(led->led_node,
> + "flash-max-timeout-us",
> + &led->max_strobe_timeout);
> + if (ret < 0) {
> + dev_warn(&led->client->dev,
> + "flash-max-timeout-us DT property missing\n");
> +
> + goto out_err;
> + }
> +
> + lm3601x_init_flash_timeout(led);
> +
> +out_err:
> + of_node_put(led->led_node);
> + return ret;
> +}
> +
> +static int lm3601x_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct lm3601x_led *led;
> + int err;
> +
> + led = devm_kzalloc(&client->dev,
> + sizeof(struct lm3601x_led), GFP_KERNEL);
> + if (!led)
> + return -ENOMEM;
> +
> + err = lm3601x_parse_node(led, client->dev.of_node);
> + if (err < 0)
> + return -ENODEV;
> +
> + led->client = client;
> + led->regmap = devm_regmap_init_i2c(client, &lm3601x_regmap);
> + if (IS_ERR(led->regmap)) {
> + err = PTR_ERR(led->regmap);
> + dev_err(&client->dev,
> + "Failed to allocate register map: %d\n", err);
> + return err;
> + }
> +
> + mutex_init(&led->lock);
> + i2c_set_clientdata(client, led);
> + err = lm3601x_register_leds(led);
> +
> + return err;
> +}
> +
> +static int lm3601x_remove(struct i2c_client *client)
> +{
> + struct lm3601x_led *led = i2c_get_clientdata(client);
> +
> + regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + LM3601X_ENABLE_MASK,
> + LM3601X_MODE_STANDBY);
You need also mutex_destroy(&led->lock) here.
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id lm3601x_id[] = {
> + { "LM36010", CHIP_LM36010 },
> + { "LM36011", CHIP_LM36011 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(i2c, lm3601x_id);
> +
> +static const struct of_device_id of_lm3601x_leds_match[] = {
> + { .compatible = "ti,lm36010", },
> + { .compatible = "ti,lm36011", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_lm3601x_leds_match);
> +
> +static struct i2c_driver lm3601x_i2c_driver = {
> + .driver = {
> + .name = "lm3601x",
> + .of_match_table = of_lm3601x_leds_match,
> + },
> + .probe = lm3601x_probe,
> + .remove = lm3601x_remove,
> + .id_table = lm3601x_id,
> +};
> +module_i2c_driver(lm3601x_i2c_driver);
> +
> +MODULE_DESCRIPTION("Texas Instruments Flash Lighting driver for LM3601X");
> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
> +MODULE_LICENSE("GPL v2");
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [patch v21 1/4] drivers: jtag: Add JTAG core driver
From: Andy Shevchenko @ 2018-05-15 21:21 UTC (permalink / raw)
To: Oleksandr Shamray
Cc: Greg Kroah-Hartman, Arnd Bergmann, Linux Kernel Mailing List,
linux-arm Mailing List, devicetree, openbmc, Joel Stanley,
Jiří Pírko, Tobias Klauser,
open list:SERIAL DRIVERS, Vadim Pasternak, system-sw-low-level,
Rob Herring, openocd-devel-owner, linux-api, David S. Miller,
Mauro Carvalho Chehab, Jiri Pirko
In-Reply-To: <1526394095-5069-2-git-send-email-oleksandrs@mellanox.com>
On Tue, May 15, 2018 at 5:21 PM, Oleksandr Shamray
<oleksandrs@mellanox.com> wrote:
> Initial patch for JTAG driver
> JTAG class driver provide infrastructure to support hardware/software
> JTAG platform drivers. It provide user layer API interface for flashing
> and debugging external devices which equipped with JTAG interface
> using standard transactions.
>
> Driver exposes set of IOCTL to user space for:
> - XFER:
> - SIR (Scan Instruction Register, IEEE 1149.1 Data Register scan);
> - SDR (Scan Data Register, IEEE 1149.1 Instruction Register scan);
> - RUNTEST (Forces the IEEE 1149.1 bus to a run state for a specified
> number of clocks).
> - SIOCFREQ/GIOCFREQ for setting and reading JTAG frequency.
>
> Driver core provides set of internal APIs for allocation and
> registration:
> - jtag_register;
> - jtag_unregister;
> - jtag_alloc;
> - jtag_free;
>
> Platform driver on registration with jtag-core creates the next
> entry in dev folder:
> /dev/jtagX
> 0xB0 all RATIO devices in development:
> <mailto:vgo@ratio.de>
> 0xB1 00-1F PPPoX <mailto:mostrows@styx.uwaterloo.ca>
> +0xB2 00-0f linux/jtag.h JTAG driver
> + <mailto:oleksandrs@mellanox.com>
Consider to preserve style (upper vs. lower).
> + This provides basic core functionality support for JTAG class devices.
> + Hardware that is equipped with a JTAG microcontroller can be
> + supported by using this driver's interfaces.
> + This driver exposes a set of IOCTLs to the user space for
> + the following commands:
> + SDR: (Scan Data Register) Performs an IEEE 1149.1 Data Register scan
> + SIR: (Scan Instruction Register) Performs an IEEE 1149.1 Instruction
> + Register scan.
> + RUNTEST: Forces the IEEE 1149.1 bus to a run state for a specified
> + number of clocks or a specified time period.
Something feels wrong with formatting here.
> +#define MAX_JTAG_NAME_LEN (sizeof("jtag") + 5)
Interesting definition. Why not to set to 10 explicitly? And why 10?
(16 sounds better)
> +struct jtag {
> + struct miscdevice miscdev;
> + struct device *dev;
Doesn't miscdev parent contain exactly this one?
> + const struct jtag_ops *ops;
> + int id;
> + bool opened;
> + struct mutex open_lock;
> + unsigned long priv[0];
> +};
> + err = copy_to_user(u64_to_user_ptr(xfer.tdio),
> + (void *)(xfer_data), data_size);
Redundant parens in one case. Check the rest similar places.
> +static int jtag_open(struct inode *inode, struct file *file)
> +{
> + struct jtag *jtag = container_of(file->private_data, struct jtag,
> + miscdev);
I would don't care about length and put it on one line.
> + if (jtag->opened) {
> + jtag->opened = true;
> + jtag->opened = false;
Can it be opened non exclusively several times? If so, this needs to
be a ref counter instead.
> + if (!ops->idle || !ops->mode_set || !ops->status_get || !ops->xfer)
> + return NULL;
Are all of them mandatory?
> +int jtag_register(struct jtag *jtag)
Perhaps devm_ variant.
> +#define jtag_u64_to_ptr(arg) ((void *)(uintptr_t)arg)
Where is this used or supposed to be used?
> +#define JTAG_MAX_XFER_DATA_LEN 65535
Is this limitation from some spec?
Otherwise why not to allow 64K?
> +/**
> + * struct jtag_ops - callbacks for jtag control functions:
> + *
> + * @freq_get: get frequency function. Filled by device driver
> + * @freq_set: set frequency function. Filled by device driver
> + * @status_get: set status function. Filled by device driver
> + * @idle: set JTAG to idle state function. Filled by device driver
> + * @xfer: send JTAG xfer function. Filled by device driver
> + */
Perhaps you need to describe which of them are _really_ mandatory and
which are optional.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 3/3] ARM: dts: tegra: Work safely with 256 MB Colibri-T20 modules
From: Stefan Agner @ 2018-05-15 21:20 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Mark Rutland, Thierry Reding, Jonathan Hunter,
devicetree, linux-tegra, linux-kernel, Marcel Ziswiler,
Lucas Stach
In-Reply-To: <1526395285-19006-4-git-send-email-krzk@kernel.org>
On 15.05.2018 16:41, Krzysztof Kozlowski wrote:
> Colibri-T20 can come in 256 MB RAM (with 512 MB NAND) or 512 MB RAM
> (with 1024 MB NAND) flavors. Both of them will use the same DTSI
> expecting the bootloader to do the fixup of /memory node. However in
> case it does not happen, let's stay on safe side by limiting the memory
> to 256 MB for both versions of Colibri-T20.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> ---
>
> RFT:
> Not tested on 512 MB module as I have only the 256 MB one.
>
> Changes since v3:
> 1. Reduce the memory in existing DTSI instead of creating a new one
> (suggested by Marcel).
>
> Changes since v2:
> 1. Do not add new compatible but use everywhere existing
> "toradex,colibri_t20-512" (suggested by Rob).
>
> Changes since v1:
> 1. Fix memory size in tegra20-colibri-256.dtsi (was working fine because
> my bootloader uses mem= argument).
> ---
> arch/arm/boot/dts/tegra20-colibri-512.dtsi | 9 +++++++--
> arch/arm/boot/dts/tegra20-iris-512.dts | 2 +-
I don't like this variant too much.
Can we not just drop the 512? And while at it, I would prefer to see the
usual $soc-$module-$carrier schema, as we use with all the other modules
of the Colibri and Apalis family.
e.g.
tegra20-colibri-512.dtsi -> tegra20-colibri.dtsi
tegra20-iris-512.dts -> tegra20-colibri-iris.dts
Yes, it is somewhat churn, but I would prefer it for the sake of clarity
and consistency...
--
Stefan
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> index 305efb275b48..b9387c540450 100644
> --- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> +++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> @@ -2,7 +2,7 @@
> #include "tegra20.dtsi"
>
> / {
> - model = "Toradex Colibri T20 512MB";
> + model = "Toradex Colibri T20 256/512 MB";
> compatible = "toradex,colibri_t20-512", "nvidia,tegra20";
>
> aliases {
> @@ -12,7 +12,12 @@
>
> memory@0 {
> device_type = "memory";
> - reg = <0x00000000 0x20000000>;
> + /*
> + * Set memory to 256 MB to be safe as this could be used on
> + * 256 or 512 MB module. It is expected from bootloader
> + * to fix this up for 512 MB version.
> + */
> + reg = <0x00000000 0x10000000>;
> };
>
> host1x@50000000 {
> diff --git a/arch/arm/boot/dts/tegra20-iris-512.dts
> b/arch/arm/boot/dts/tegra20-iris-512.dts
> index 40126388946d..efaa96228722 100644
> --- a/arch/arm/boot/dts/tegra20-iris-512.dts
> +++ b/arch/arm/boot/dts/tegra20-iris-512.dts
> @@ -4,7 +4,7 @@
> #include "tegra20-colibri-512.dtsi"
>
> / {
> - model = "Toradex Colibri T20 512MB on Iris";
> + model = "Toradex Colibri T20 256/512 MB on Iris";
> compatible = "toradex,iris", "toradex,colibri_t20-512", "nvidia,tegra20";
>
> aliases {
^ permalink raw reply
* Re: [PATCH v6 1/2] dt: bindings: lm3601x: Introduce the lm3601x driver
From: Jacek Anaszewski @ 2018-05-15 21:13 UTC (permalink / raw)
To: Dan Murphy, robh+dt, mark.rutland, pavel
Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <20180515154352.20263-1-dmurphy@ti.com>
Hi Dan,
Thanks for the update.
On 05/15/2018 05:43 PM, Dan Murphy wrote:
> Introduce the device tree bindings for the lm3601x
> family of LED torch, flash and IR drivers.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>
> v6 - Removed multiple led child nodes, fixed example to display micro ranges
> for corresponding child nodes and added led-sources to define the current driver -
> https://patchwork.kernel.org/patch/10392121/
>
> v5 - No changes - https://patchwork.kernel.org/patch/10391743/
> v4 - Added " " around "=", changed strobe to flash on label, removed "support and
> register" comment and change ir lable to ir:torch - See v2 patchworks for comments
> v3 - Removed wildcard compatible - https://patchwork.kernel.org/patch/10386241/
> v2 - No changes - https://patchwork.kernel.org/patch/10384587/
>
> .../devicetree/bindings/leds/leds-lm3601x.txt | 47 +++++++++++++++++++
> 1 file changed, 47 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3601x.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3601x.txt b/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
> new file mode 100644
> index 000000000000..27930a89e9a5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
> @@ -0,0 +1,47 @@
> +* Texas Instruments - lm3601x Single-LED Flash Driver
> +
> +The LM3601X are ultra-small LED flash drivers that
> +provide a high level of adjustability.
> +
> +Required properties:
> + - compatible : Can be one of the following
> + "ti,lm36010"
> + "ti,lm36011"
> + - reg : I2C slave address
> + - #address-cells : 1
> + - #size-cells : 0
> +
> +Required child properties:
> + - reg : 0
> + - led-sources: 0 - Indicates a IR mode
> + 1 - Indicates a Torch (white LED) mode
You don't need led-sources at all. Please use reg as in
the previous version. led-sources is useful for describing
more complex arrangements. Here reg will do.
> +
> +Required properties for flash LED child nodes:
> + See Documentation/devicetree/bindings/leds/common.txt
> + - flash-max-microamp : Range from 11mA -> 1.5A
> + - flash-max-timeout-us : Range from 40ms -> 1600ms
> + - led-max-microamp : Range from 2.4mA -> 376mA
Please give also a step in the format like below
(taken from max77693):
Valid values: 62500 - 1000000, step by 62500 (rounded down)
> +
> +Optional child properties:
> + - label : see Documentation/devicetree/bindings/leds/common.txt
> +
> +Example:
> +led-controller@64 {
> + compatible = "ti,lm36010";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x64>;
> +
> + led@0 {
> + reg = <0>;
> + label = "white:torch";
> + led-max-microamp = <376000>;
> + flash-max-microamp = <1500000>;
> + flash-max-timeout-us = <1600000>;
> + led-sources = <1>;
> + };
> +}
> +
> +For more product information please see the links below:
> +http://www.ti.com/product/LM36010
> +http://www.ti.com/product/LM36011
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [patch v21 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx families JTAG master driver
From: Andy Shevchenko @ 2018-05-15 21:00 UTC (permalink / raw)
To: Oleksandr Shamray
Cc: Greg Kroah-Hartman, Arnd Bergmann, Linux Kernel Mailing List,
linux-arm Mailing List, devicetree, openbmc, Joel Stanley,
Jiří Pírko, Tobias Klauser,
open list:SERIAL DRIVERS, Vadim Pasternak, system-sw-low-level,
Rob Herring, openocd-devel-owner, linux-api, David S. Miller,
Mauro Carvalho Chehab, Jiri Pirko
In-Reply-To: <1526394095-5069-3-git-send-email-oleksandrs@mellanox.com>
On Tue, May 15, 2018 at 5:21 PM, Oleksandr Shamray
<oleksandrs@mellanox.com> wrote:
> Driver adds support of Aspeed 2500/2400 series SOC JTAG master controller.
>
> Driver implements the following jtag ops:
> - freq_get;
> - freq_set;
> - status_get;
> - idle;
> - xfer;
>
> It has been tested on Mellanox system with BMC equipped with
> Aspeed 2520 SoC for programming CPLD devices.
> +#define ASPEED_JTAG_DATA 0x00
> +#define ASPEED_JTAG_INST 0x04
> +#define ASPEED_JTAG_CTRL 0x08
> +#define ASPEED_JTAG_ISR 0x0C
> +#define ASPEED_JTAG_SW 0x10
> +#define ASPEED_JTAG_TCK 0x14
> +#define ASPEED_JTAG_EC 0x18
> +
> +#define ASPEED_JTAG_DATA_MSB 0x01
> +#define ASPEED_JTAG_DATA_CHUNK_SIZE 0x20
> +#define ASPEED_JTAG_IOUT_LEN(len) (ASPEED_JTAG_CTL_ENG_EN |\
> + ASPEED_JTAG_CTL_ENG_OUT_EN |\
> + ASPEED_JTAG_CTL_INST_LEN(len))
Better to read
#define MY_COOL_CONST_OR_MACRO(xxx) \
...
> +#define ASPEED_JTAG_DOUT_LEN(len) (ASPEED_JTAG_CTL_ENG_EN |\
> + ASPEED_JTAG_CTL_ENG_OUT_EN |\
> + ASPEED_JTAG_CTL_DATA_LEN(len))
Ditto.
> +static char *end_status_str[] = {"idle", "ir pause", "drpause"};
> +static int aspeed_jtag_freq_set(struct jtag *jtag, u32 freq)
> +{
> + struct aspeed_jtag *aspeed_jtag = jtag_priv(jtag);
> + unsigned long apb_frq;
> + u32 tck_val;
> + u16 div;
> +
> + apb_frq = clk_get_rate(aspeed_jtag->pclk);
> + div = (apb_frq % freq == 0) ? (apb_frq / freq) - 1 : (apb_frq / freq);
Isn't it the same as
div = (apb_frq - 1) / freq;
?
> + tck_val = aspeed_jtag_read(aspeed_jtag, ASPEED_JTAG_TCK);
> + aspeed_jtag_write(aspeed_jtag,
> + (tck_val & ASPEED_JTAG_TCK_DIVISOR_MASK) | div,
> + ASPEED_JTAG_TCK);
> + return 0;
> +}
> +static void aspeed_jtag_sw_delay(struct aspeed_jtag *aspeed_jtag, int cnt)
> +{
> + int i;
> +
> + for (i = 0; i < cnt; i++)
> + aspeed_jtag_read(aspeed_jtag, ASPEED_JTAG_SW);
Isn't it readsl() (or how it's called, I don't remember).
> +}
> +static void aspeed_jtag_wait_instruction_pause(struct aspeed_jtag *aspeed_jtag)
> +{
> + wait_event_interruptible(aspeed_jtag->jtag_wq, aspeed_jtag->flag &
> + ASPEED_JTAG_ISR_INST_PAUSE);
In such cases I prefer to see a new line with a parameter in full.
Check all places.
> + aspeed_jtag->flag &= ~ASPEED_JTAG_ISR_INST_PAUSE;
> +}
> +static void aspeed_jtag_sm_cycle(struct aspeed_jtag *aspeed_jtag, const u8 *tms,
> + int len)
> +{
> + int i;
> +
> + for (i = 0; i < len; i++)
> + aspeed_jtag_tck_cycle(aspeed_jtag, tms[i], 0);
> +}
> +
> +static void aspeed_jtag_run_test_idle_sw(struct aspeed_jtag *aspeed_jtag,
> + struct jtag_run_test_idle *runtest)
> +{
> + static const u8 sm_pause_irpause[] = {1, 1, 1, 1, 0, 1, 0};
> + static const u8 sm_pause_drpause[] = {1, 1, 1, 0, 1, 0};
> + static const u8 sm_idle_irpause[] = {1, 1, 0, 1, 0};
> + static const u8 sm_idle_drpause[] = {1, 0, 1, 0};
> + static const u8 sm_pause_idle[] = {1, 1, 0};
> + int i;
> +
> + /* SW mode from idle/pause-> to pause/idle */
> + if (runtest->reset) {
> + for (i = 0; i < ASPEED_JTAG_RESET_CNTR; i++)
> + aspeed_jtag_tck_cycle(aspeed_jtag, 1, 0);
> + }
I would rather split this big switch to a few helper functions per
each status of surrounding switch.
> +
> + /* Stay on IDLE for at least TCK cycle */
> + for (i = 0; i < runtest->tck; i++)
> + aspeed_jtag_tck_cycle(aspeed_jtag, 0, 0);
> +}
> +/**
> + * aspeed_jtag_run_test_idle:
> + * JTAG reset: generates at least 9 TMS high and 1 TMS low to force
> + * devices into Run-Test/Idle State.
> + */
It's rather broken kernel doc.
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_CTL_ENG_EN |
> + ASPEED_JTAG_CTL_ENG_OUT_EN |
> + ASPEED_JTAG_CTL_FORCE_TMS, ASPEED_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_EC_GO_IDLE,
> + ASPEED_JTAG_EC);
> + aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_SW_MODE_EN |
> + ASPEED_JTAG_SW_MODE_TDIO, ASPEED_JTAG_SW);
Here you have permutations of flag some of which are repeatetive in
the code. Perhaps make additional definitions instead.
Check other similar places.
> + char tdo;
Indentation.
> + if (xfer->direction == JTAG_READ_XFER)
> + tdi = UINT_MAX;
> + else
> + tdi = data[index];
> + if (xfer->direction == JTAG_READ_XFER)
> + tdi = UINT_MAX;
> + else
> + tdi = data[index];
Take your time to think how the above duplication can be avoided.
> + }
> + }
> +
> + tdo = aspeed_jtag_tck_cycle(aspeed_jtag, 1, tdi & ASPEED_JTAG_DATA_MSB);
> + data[index] |= tdo << (shift_bits % ASPEED_JTAG_DATA_CHUNK_SIZE);
> +}
> + if (endstate != JTAG_STATE_IDLE) {
Why not to use positive check?
> + int i;
> +
> + for (i = 0; i <= xfer->length / BITS_PER_BYTE; i++) {
> + pos += snprintf(&dbg_str[pos], sizeof(dbg_str) - pos,
> + "0x%02x ", xfer_data[i]);
> + }
Oh, NO! Consider reading printk-formats (for %*ph) and other
documentation about available APIs.
> + if (!(aspeed_jtag->mode & JTAG_XFER_HW_MODE)) {
> + /* SW mode */
This is rather too complex to be in one function.
> + } else {
> + /* hw mode */
> + aspeed_jtag_write(aspeed_jtag, 0, ASPEED_JTAG_SW);
> + aspeed_jtag_xfer_hw(aspeed_jtag, xfer, data);
For symmetry it might be another function.
> + }
> + dev_dbg(aspeed_jtag->dev, "status %x\n", status);
Perhaps someone should become familiar with tracepoints?
> + dev_err(aspeed_jtag->dev, "irq status:%x\n",
> + status);
Huh, really?! SPAM.
(I would drop it completely, though you may use ratelimited variant)
> + ret = IRQ_NONE;
> + }
> + return ret;
> +}
> + clk_prepare_enable(aspeed_jtag->pclk);
This might fail.
> + dev_dbg(&pdev->dev, "IRQ %d.\n", aspeed_jtag->irq);
Noise even for debug.
> + err = jtag_register(jtag);
Perhaps we might have devm_ variant of this. Check how SPI framework
deal with a such.
> +static int aspeed_jtag_remove(struct platform_device *pdev)
> +{
> + struct jtag *jtag;
> +
> + jtag = platform_get_drvdata(pdev);
Usually we put this on one line
> + aspeed_jtag_deinit(pdev, jtag_priv(jtag));
> + jtag_unregister(jtag);
> + jtag_free(jtag);
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port power and data config
From: Mats Karrman @ 2018-05-15 20:52 UTC (permalink / raw)
To: Jun Li, robh+dt@kernel.org, gregkh@linuxfoundation.org,
heikki.krogerus@linux.intel.com, linux@roeck-us.net
Cc: a.hajda@samsung.com, cw00.choi@samsung.com,
shufan_lee@richtek.com, Peter Chen, gsomlo@gmail.com,
devicetree@vger.kernel.org, linux-usb@vger.kernel.org,
dl-linux-imx
In-Reply-To: <VI1PR0402MB3917CE86B45CBC1B6F7F576B899C0@VI1PR0402MB3917.eurprd04.prod.outlook.com>
Hi,
On 05/14/2018 11:36 AM, Jun Li wrote:
> Hi
>> -----Original Message-----
>> From: Mats Karrman [mailto:mats.dev.list@gmail.com]
>> Sent: 2018年5月12日 3:56
>> To: Jun Li <jun.li@nxp.com>; robh+dt@kernel.org; gregkh@linuxfoundation.org;
>> heikki.krogerus@linux.intel.com; linux@roeck-us.net
>> Cc: a.hajda@samsung.com; cw00.choi@samsung.com;
>> shufan_lee@richtek.com; Peter Chen <peter.chen@nxp.com>;
>> gsomlo@gmail.com; devicetree@vger.kernel.org; linux-usb@vger.kernel.org;
>> dl-linux-imx <linux-imx@nxp.com>
>> Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port power
>> and data config
>>
>> Hi Li Jun,
>>
>> On 2018-05-03 02:24, Li Jun wrote:
>>
>>> This patch adds 3 APIs to get the typec port power and data type, and
>>> preferred power role by its name string.
>>>
>>> Signed-off-by: Li Jun <jun.li@nxp.com>
>>> ---
>>> drivers/usb/typec/class.c | 52
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/usb/typec.h | 3 +++
>>> 2 files changed, 55 insertions(+)
>>>
>>> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
>>> index 53df10d..5981e18 100644
>>> --- a/drivers/usb/typec/class.c
>>> +++ b/drivers/usb/typec/class.c
>>> @@ -9,6 +9,7 @@
>>> #include <linux/device.h>
>>> #include <linux/module.h>
>>> #include <linux/mutex.h>
>>> +#include <linux/property.h>
>>> #include <linux/slab.h>
>>> #include <linux/usb/typec.h>
>>> #include <linux/usb/typec_mux.h>
>>> @@ -802,6 +803,12 @@ static const char * const typec_port_types[] = {
>>> [TYPEC_PORT_DRP] = "dual",
>>> };
>>>
>>> +static const char * const typec_data_types[] = {
>>> + [TYPEC_PORT_DFP] = "host",
>>> + [TYPEC_PORT_UFP] = "device",
>>> + [TYPEC_PORT_DRD] = "dual",
>>> +};
>>> +
>>> static const char * const typec_port_types_drp[] = {
>>> [TYPEC_PORT_SRC] = "dual [source] sink",
>>> [TYPEC_PORT_SNK] = "dual source [sink]", @@ -1252,6 +1259,51
>> @@
>>> void typec_set_pwr_opmode(struct typec_port *port,
>>> }
>>> EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
>>>
>>> +/**
>>> + * typec_find_power_type - Get the typec port power type
>> Why is this function called typec_find_power_type() and not
>> typec_find_port_type()?
>> It's called port_type in sysfs, having different names just adds confusion.
>> (Otherwise I agree power_type is a better name but...)
> We have "port type" before the power and data role separation,
> this API name's intention is to reflect the power cap, anyway I
> leave this to be decided by Heikki then.
>>> + * @name: port type string
>>> + *
>>> + * This routine is used to find the typec_port_type by its string name.
>>> + *
>>> + * Returns typec_port_type if success, otherwise negative error code.
>>> + */
>>> +int typec_find_power_type(const char *name) {
>>> + return match_string(typec_port_types, ARRAY_SIZE(typec_port_types),
>>> + name);
>>> +}
>>> +EXPORT_SYMBOL_GPL(typec_find_power_type);
>>> +
>>> +/**
>>> + * typec_find_preferred_role - Find the typec drp port preferred
>>> +power role
>> Why typec_find_preferred_role()? Could be used for any power_role so why not
>> typec_find_power_role()?
> I am not sure if I catch your point of this comment.
> For preferred role(if support try.sink or try.src) the only allowed power roles are
> "sink"
> "source"
> But for power role, the allowed type are
> "sink"
> "source"
> "dual"
Uhm, typing too fast again, I am. A better name would be just typec_find_role().
What I mean is that the function could be used for any situation when
someone wants to map a string to a TYPEC_{SOURCE,SINK} constant so it
is unnecessary to limit its usage to just preferred role.
// Mats
> Thanks
> Li Jun
>> BR // Mats
>>
>>> + * @name: power role string
>>> + *
>>> + * This routine is used to find the typec_role by its string name of
>>> + * preferred power role(Try.SRC or Try.SNK).
>>> + *
>>> + * Returns typec_role if success, otherwise negative error code.
>>> + */
>>> +int typec_find_preferred_role(const char *name) {
>>> + return match_string(typec_roles, ARRAY_SIZE(typec_roles), name); }
>>> +EXPORT_SYMBOL_GPL(typec_find_preferred_role);
>>> +
>>> +/**
>>> + * typec_find_data_type - Get the typec port data capability
>>> + * @name: data type string
>>> + *
>>> + * This routine is used to find the typec_port_data by its string name.
>>> + *
>>> + * Returns typec_port_data if success, otherwise negative error code.
>>> + */
>>> +int typec_find_data_type(const char *name) {
>>> + return match_string(typec_data_types, ARRAY_SIZE(typec_data_types),
>>> + name);
>>> +}
>>> +EXPORT_SYMBOL_GPL(typec_find_data_type);
>>> +
>>> /* ------------------------------------------ */
>>> /* API for Multiplexer/DeMultiplexer Switches */
>>>
>>> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
>>> index 672b39b..00c93e7 100644
>>> --- a/include/linux/usb/typec.h
>>> +++ b/include/linux/usb/typec.h
>>> @@ -267,4 +267,7 @@ int typec_set_orientation(struct typec_port *port,
>>> enum typec_orientation orientation);
>>> int typec_set_mode(struct typec_port *port, int mode);
>>>
>>> +int typec_find_power_type(const char *name); int
>>> +typec_find_preferred_role(const char *name); int
>>> +typec_find_data_type(const char *name);
>>> #endif /* __LINUX_USB_TYPEC_H */
^ permalink raw reply
* Re: [PATCH][next] scripts/dtc: fix spelling mistake: "endpont" -> "endpoint"
From: Frank Rowand @ 2018-05-15 20:42 UTC (permalink / raw)
To: Colin King, Rob Herring, devicetree; +Cc: kernel-janitors, linux-kernel
In-Reply-To: <20180515090209.9165-1-colin.king@canonical.com>
Hi Colin,
On 05/15/18 02:02, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in FAIL message.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> scripts/dtc/checks.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/dtc/checks.c b/scripts/dtc/checks.c
> index a2cc1036c915..9d253819e12c 100644
> --- a/scripts/dtc/checks.c
> +++ b/scripts/dtc/checks.c
> @@ -1536,7 +1536,7 @@ static void check_graph_endpoint(struct check *c, struct dt_info *dti,
> return;
>
> if (!strprefixeq(node->name, node->basenamelen, "endpoint"))
> - FAIL(c, dti, node, "graph endpont node name should be 'endpoint'");
> + FAIL(c, dti, node, "graph endpoint node name should be 'endpoint'");
>
> check_graph_reg(c, dti, node);
>
>
Thanks for catching this.
We import this file from the upstream dtc project. Please send this same exact patch
to:
David Gibson <david@gibson.dropbear.id.au>
devicetree-compiler@vger.kernel.org
Thanks,
Frank
^ permalink raw reply
* RE: [PATCH v6 04/11] firmware: xilinx: Add query data API
From: Jolly Shah @ 2018-05-15 20:29 UTC (permalink / raw)
To: Sudeep Holla, ard.biesheuvel@linaro.org, mingo@kernel.org,
gregkh@linuxfoundation.org, matt@codeblueprint.co.uk,
hkallweit1@gmail.com, keescook@chromium.org,
dmitry.torokhov@gmail.com, mturquette@baylibre.com,
sboyd@codeaurora.org, michal.simek@xilinx.com, robh+dt@kernel.org,
mark.rutland@arm.com, linux-clk@vger.kernel.org
Cc: Rajan Vaja, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <ab9632f5-882d-462d-d830-0316cad56baf@arm.com>
Hi Sudeep,
> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> Sent: Tuesday, May 15, 2018 2:34 AM
> To: Jolly Shah <JOLLYS@xilinx.com>; ard.biesheuvel@linaro.org;
> mingo@kernel.org; gregkh@linuxfoundation.org; matt@codeblueprint.co.uk;
> hkallweit1@gmail.com; keescook@chromium.org;
> dmitry.torokhov@gmail.com; mturquette@baylibre.com;
> sboyd@codeaurora.org; michal.simek@xilinx.com; robh+dt@kernel.org;
> mark.rutland@arm.com; linux-clk@vger.kernel.org
> Cc: Sudeep Holla <sudeep.holla@arm.com>; Rajan Vaja <RAJANV@xilinx.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH v6 04/11] firmware: xilinx: Add query data API
>
>
>
> On 14/05/18 20:16, Jolly Shah wrote:
> > HI Sudeep,
> >
>
> [...]
>
> >>
> >> Can you give more insight into this ? How will be this used ? How
> >> this aligns with data we get from DT ? I am just trying to understand
> >> how is this information split between this API and DT for example.
> >>
> >
> > This API is used to get clock information from firmware and register
> > clocks accordingly in driver. In our case, firmware maintains database
> > of all clocks available on chip. DT will provide information for off
> > chip reference clocks only. This is to avoid duplication of clocks
> > data in DT and firmware both as firmware anyways need clock data to
> > manage them.
> >
>
> I wanted to understand the difference with example. What kind of information
> you get from DT and what you get from firmware ?
>
> --
> Regards,
> Sudeep
Below is an example showing PLL and Leaf clock derivation:
Input ref clocks->Mux->Multiplier/Divider->PLL*
Pll1/pll2/pll3/pll4->Mux->Divider->Gate->Leaf clock1
Here Off chip input ref clock information comes from DT.
Rest information for Pll/leaf clocks come from firmware using query API. For clock driver, query API is used to get topology, flags, parents etc information per clock.
Thanks,
Jolly Shah
^ permalink raw reply
* Re: [PATCH v2 09/11] docs: Fix some broken references
From: Stephen Boyd @ 2018-05-15 20:26 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Catalin Marinas, Will Deacon, dri-devel, Jaroslav Kysela,
Eric Paris, Mauro Carvalho Chehab, linux-clk, James Morris,
Alan Stern, xen-devel, Boqun Feng, Nicholas Piggin,
Thomas Gleixner, Antoine Jacquet, Greg Kroah-Hartman, linux-usb,
linux-kernel, Li Zefan, linux-crypto, Mark Rutland, alsa-devel,
David Airlie, Max Filippov, Harry Wei, selinux
In-Reply-To: <e959f23d6f6905ee606fadfda13e2bb37deed017.1525870886.git.mchehab+samsung@kernel.org>
Quoting Mauro Carvalho Chehab (2018-05-09 06:18:52)
> As we move stuff around, some doc references are broken. Fix some of
> them via this script:
> ./scripts/documentation-file-ref-check --fix-rst
>
> Manually checked if the produced result is valid, removing a few
> false-positives.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
> drivers/clk/clk.c | 4 ++--
> drivers/clk/ingenic/cgu.h | 2 +-
Acked-by: Stephen Boyd <sboyd@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v2] Add support for external reference voltage through the regulator framework
From: Silvan Murer @ 2018-05-15 20:14 UTC (permalink / raw)
To: lars, jic23; +Cc: linux-iio, devicetree
Add support for external reference voltage through the regulator framework.
Signed-off-by: Silvan Murer <silvan.murer@gmail.com>
---
Changes in v2:
- Add 'optional properties' documentation
- Return an error when a regulator is specified
- Use internal reference when no regulator is specified
- Use iio_device_register instead of devm_iio_device_register
.../devicetree/bindings/iio/dac/ltc2632.txt | 14 +++++
drivers/iio/dac/ltc2632.c | 70 +++++++++++++++++++---
2 files changed, 75 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/dac/ltc2632.txt b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
index eb911e5..e0d5fea 100644
--- a/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
+++ b/Documentation/devicetree/bindings/iio/dac/ltc2632.txt
@@ -12,12 +12,26 @@ Required properties:
Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
apply. In particular, "reg" and "spi-max-frequency" properties must be given.
+Optional properties:
+ - vref-supply: Phandle to the external reference voltage supply. This should
+ only be set if there is an external reference voltage connected to the VREF
+ pin. If the property is not set the internal reference is used.
+
Example:
+ vref: regulator-vref {
+ compatible = "regulator-fixed";
+ regulator-name = "vref-ltc2632";
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1250000>;
+ regulator-always-on;
+ };
+
spi_master {
dac: ltc2632@0 {
compatible = "lltc,ltc2632-l12";
reg = <0>; /* CS0 */
spi-max-frequency = <1000000>;
+ vref-supply = <&vref>; /* optional */
};
};
diff --git a/drivers/iio/dac/ltc2632.c b/drivers/iio/dac/ltc2632.c
index d322b78..7127cf6 100644
--- a/drivers/iio/dac/ltc2632.c
+++ b/drivers/iio/dac/ltc2632.c
@@ -2,6 +2,7 @@
* LTC2632 Digital to analog convertors spi driver
*
* Copyright 2017 Maxime Roussin-Bélanger
+ * expanded by Silvan Murer <silvan.murer@gmail.com>
*
* Licensed under the GPL-2.
*/
@@ -10,6 +11,7 @@
#include <linux/spi/spi.h>
#include <linux/module.h>
#include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
#define LTC2632_DAC_CHANNELS 2
@@ -28,7 +30,7 @@
/**
* struct ltc2632_chip_info - chip specific information
* @channels: channel spec for the DAC
- * @vref_mv: reference voltage
+ * @vref_mv: internal reference voltage
*/
struct ltc2632_chip_info {
const struct iio_chan_spec *channels;
@@ -39,10 +41,14 @@ struct ltc2632_chip_info {
* struct ltc2632_state - driver instance specific data
* @spi_dev: pointer to the spi_device struct
* @powerdown_cache_mask used to show current channel powerdown state
+ * @vref_mv used reference voltage (internal or external)
+ * @vref_reg regulator for the reference voltage
*/
struct ltc2632_state {
struct spi_device *spi_dev;
unsigned int powerdown_cache_mask;
+ int vref_mv;
+ struct regulator *vref_reg;
};
enum ltc2632_supported_device_ids {
@@ -90,7 +96,7 @@ static int ltc2632_read_raw(struct iio_dev *indio_dev,
switch (m) {
case IIO_CHAN_INFO_SCALE:
- *val = chip_info->vref_mv;
+ *val = st->vref_mv;
*val2 = chan->scan_type.realbits;
return IIO_VAL_FRACTIONAL_LOG2;
}
@@ -247,6 +253,45 @@ static int ltc2632_probe(struct spi_device *spi)
chip_info = (struct ltc2632_chip_info *)
spi_get_device_id(spi)->driver_data;
+ st->vref_reg = devm_regulator_get_optional(&spi->dev, "vref");
+ if (PTR_ERR(st->vref_reg) == -ENODEV) {
+ /* use internal reference voltage */
+ st->vref_reg = NULL;
+ st->vref_mv = chip_info->vref_mv;
+
+ ret = ltc2632_spi_write(spi, LTC2632_CMD_INTERNAL_REFER,
+ 0, 0, 0);
+ if (ret) {
+ dev_err(&spi->dev,
+ "Set internal reference command failed, %d\n",
+ ret);
+ return ret;
+ }
+ } else if (IS_ERR(st->vref_reg)) {
+ dev_err(&spi->dev,
+ "Error getting voltage reference regulator\n");
+ return PTR_ERR(st->vref_reg);
+ } else {
+ /* use external reference voltage */
+ ret = regulator_enable(st->vref_reg);
+ if (ret) {
+ dev_err(&spi->dev,
+ "enable reference regulator failed, %d\n",
+ ret);
+ return ret;
+ }
+ st->vref_mv = regulator_get_voltage(st->vref_reg) / 1000;
+
+ ret = ltc2632_spi_write(spi, LTC2632_CMD_EXTERNAL_REFER,
+ 0, 0, 0);
+ if (ret) {
+ dev_err(&spi->dev,
+ "Set external reference command failed, %d\n",
+ ret);
+ return ret;
+ }
+ }
+
indio_dev->dev.parent = &spi->dev;
indio_dev->name = dev_of_node(&spi->dev) ? dev_of_node(&spi->dev)->name
: spi_get_device_id(spi)->name;
@@ -255,14 +300,20 @@ static int ltc2632_probe(struct spi_device *spi)
indio_dev->channels = chip_info->channels;
indio_dev->num_channels = LTC2632_DAC_CHANNELS;
- ret = ltc2632_spi_write(spi, LTC2632_CMD_INTERNAL_REFER, 0, 0, 0);
- if (ret) {
- dev_err(&spi->dev,
- "Set internal reference command failed, %d\n", ret);
- return ret;
- }
+ return iio_device_register(indio_dev);
+}
+
+static int ltc2632_remove(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev = spi_get_drvdata(spi);
+ struct ltc2632_state *st = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+
+ if (st->vref_reg)
+ regulator_disable(st->vref_reg);
- return devm_iio_device_register(&spi->dev, indio_dev);
+ return 0;
}
static const struct spi_device_id ltc2632_id[] = {
@@ -306,6 +357,7 @@ static struct spi_driver ltc2632_driver = {
.of_match_table = of_match_ptr(ltc2632_of_match),
},
.probe = ltc2632_probe,
+ .remove = ltc2632_remove,
.id_table = ltc2632_id,
};
module_spi_driver(ltc2632_driver);
--
2.7.4
^ permalink raw reply related
* Re: [PATCHv2] arm64: dts: stratix10: Add QSPI support for Stratix10
From: Thor Thayer @ 2018-05-15 20:10 UTC (permalink / raw)
To: Dinh Nguyen, robh+dt, mark.rutland
Cc: catalin.marinas, will.deacon, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20b33e1d-0609-78dd-dc9c-922658a11f18@kernel.org>
On 05/15/2018 10:11 AM, Dinh Nguyen wrote:
>
> On 05/11/2018 11:10 AM, thor.thayer@linux.intel.com wrote:
>> From: Thor Thayer <thor.thayer@linux.intel.com>
>>
>> Add qspi_clock
>> The qspi_clk frequency is updated by U-Boot before starting Linux.
>> Add QSPI interface node.
>> Add QSPI flash memory child node.
>> Setup the QSPI memory in 2 partitions.
>>
>> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
>> ---
>> v2 s/_/-/ in qspi-clk
>> rename flash node.
>> use partition child node notation<snip>>> + qspi: spi@ff8d2000 {
>> + compatible = "cdns,qspi-nor";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0xff8d2000 0x100>,
>> + <0xff900000 0x100000>;
>> + interrupts = <0 3 4>;
>> + cdns,fifo-depth = <128>;
>> + cdns,fifo-width = <4>;
>> + cdns,trigger-address = <0x00000000>;
>> + clocks = <&qspi_clk>;
>> + bus-num = <1>;
>
> I don't you need "bus-num" anymore right? I don't see it getting used
> anywhere in the driver.
>
> Dinh
>
Yes, I missed that holdover. Thanks, I'll correct that.
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: am437x-sk-evm: add wilink8 support
From: Tony Lindgren @ 2018-05-15 18:32 UTC (permalink / raw)
To: Eyal Reizer
Cc: bcousson, robh+dt, mark.rutland, linux-omap, devicetree,
linux-kernel, nsekhar, Eyal Reizer
In-Reply-To: <1525593486-13028-1-git-send-email-eyalr@ti.com>
* Eyal Reizer <eyalreizer@gmail.com> [180506 01:00]:
> enable mmc3 used for wlan and uart1 used for bluetooth
> configure the gpios used for wlan and bluetooth controls
> add fixed voltage regulator used for wlan power control
Applying into omap-for-v4.18/dt thanks.
Tony
^ permalink raw reply
* Re: [PATCH] ARM: dts: am335x-baltos-ir3220: Use IRQ_TYPE specifier
From: Tony Lindgren @ 2018-05-15 18:18 UTC (permalink / raw)
To: Hernán Gonzalez
Cc: bcousson, robh+dt, mark.rutland, linux-omap, devicetree
In-Reply-To: <1526253393-3247-1-git-send-email-hernan@vanguardiasur.com.ar>
* Hernán Gonzalez <hernan@vanguardiasur.com.ar> [180513 23:19]:
> GPIO_ACTIVE_LOW was being used to specify an interrupt, use
> IRQ_TYPE_EDGE_RISING instead. This improves DT readability.
Thanks applying all five similar patchs into omap-for-v4.18/dt.
Tony
^ permalink raw reply
* Re: [PATCH 0/4] ARM: dts: am437x boards: Correct (again) tps65218 irq type
From: Tony Lindgren @ 2018-05-15 18:15 UTC (permalink / raw)
To: Keerthy; +Cc: Peter Ujfalusi, bcousson, linux-omap, devicetree, linux-kernel
In-Reply-To: <4b4100d0-85e1-659b-8f7e-b3d4b3925e82@ti.com>
* Keerthy <j-keerthy@ti.com> [180509 04:03]:
>
>
> On Tuesday 08 May 2018 07:46 PM, Tony Lindgren wrote:
> > * Peter Ujfalusi <peter.ujfalusi@ti.com> [180508 13:22]:
> >> Hi,
> >>
> >> While based on the datasheet of tps65218 the INT is low active, the GIC_SPI
> >> does not support anythin but IRQ_TYPE_LEVEL_HIGH or IRQ_TYPE_EDGE_RISING:
> >>
> >> [ 2.761814] genirq: Setting trigger mode 8 for irq 102 failed (irq_chip_set_type_parent+0x0/0x30)
> >> [ 2.770913] tps65218 0-0024: Failed to request IRQ 102 for tps65218: -22
> >> [ 2.777854] tps65218: probe of 0-0024 failed with error -22
> >
> > So does the tps65218 have some register to control the interrupt
> > direction or is it's datasheet wrong?
> >
> > BTW, ADC might be a good test case for PMIC interrupt if it has one.
>
> There is no ADC on this PMIC.
>
> >
> >> Use LEVEL_HIGH for the interrupt as it looks to be the correct setting:
> >> INTn of tps65218 is connected to NMIn of the SoC.
> >>
> >> The offending patches are only in linux-next.
> >
> > OK
Applying these all into omap-for-v4.18/dt.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] ARM: dts: am335x-evmsk: Add phandle for the backlight for the panel
From: Tony Lindgren @ 2018-05-15 18:14 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: devicetree, linux-omap, bcousson, linux-arm-kernel
In-Reply-To: <20180508100433.1051-1-peter.ujfalusi@ti.com>
* Peter Ujfalusi <peter.ujfalusi@ti.com> [180508 03:05]:
> With the backlight phandle the driver can manage the backlight on/off in
> sync with the panel enable/disable.
Applying into omap-for-v4.18/dt thanks.
Tony
^ permalink raw reply
* Re: [PATCH] arm: dts: omap: fix OMAP3 CM-T3x OF graph video connectors
From: Tony Lindgren @ 2018-05-15 18:11 UTC (permalink / raw)
To: Rob Herring; +Cc: devicetree, Benoît Cousson, linux-arm-kernel
In-Reply-To: <20180508135930.5768-6-robh@kernel.org>
* Rob Herring <robh@kernel.org> [180508 07:01]:
> The OMAP3 CM-T3x based boards define 2 /connector nodes for S-Video and
> DVI output. However, since they have the same node name, the S-Video
> connector overwritten. This leaves a dangling graph connection which
> gives the following warning:
>
> arch/arm/boot/dts/omap3-sbc-t3517.dtb: Warning (graph_endpoint): /ocp@68000000/dss@48050000/encoder@48050c00/port/endpoint: graph connection to node '/connector/port/endpoint' is not bidirectional
>
> Fix this by renaming the nodes to s-video-connector and dvi-connector.
Applying this too into omap-for-v4.18/dt-fixes.
Regards,
Tony
^ permalink raw reply
* Re: [RESEND PATCH v2 1/1] net: phy: micrel: add 125MHz reference clock workaround
From: Markus Niebel @ 2018-05-15 18:11 UTC (permalink / raw)
To: Marco Felsch, robh+dt, mark.rutland, andrew, f.fainelli
Cc: netdev, devicetree, kernel, Markus.Niebel
In-Reply-To: <20180515081856.23322-2-m.felsch@pengutronix.de>
Am Dienstag, den 15.05.2018, 10:18 +0200 schrieb Marco Felsch:
> From: Markus Niebel <Markus.Niebel@tqs.de>
>
> The micrel KSZ9031 phy has a optional clock pin (CLK125_NDO) which can be
> used as reference clock for the MAC unit. The clock signal must meet the
> RGMII requirements to ensure the correct data transmission between the
> MAC and the PHY. The KSZ9031 phy does not fulfill the duty cycle
> requirement if the phy is configured as slave. For a complete
> describtion look at the errata sheets: DS80000691D or DS80000692D.
>
> The errata sheet recommends to force the phy into master mode whenever
> there is a 1000Base-T link-up as work around. Only set the
> "micrel,force-master" property if you use the phy reference clock provided
> by CLK125_NDO pin as MAC reference clock in your application.
>
> Attenation, this workaround is only usable if the link partner can
> be configured to slave mode for 1000Base-T.
>
Thank you for documenting and bringing it mainline, should be clear enough what the
modification is meant for.
Markus
^ permalink raw reply
* Re: [PATCH] arm: dts: omap: fix OF graph in omap3-devkit8000
From: Tony Lindgren @ 2018-05-15 18:11 UTC (permalink / raw)
To: Rob Herring; +Cc: devicetree, Benoît Cousson, linux-arm-kernel
In-Reply-To: <20180508135930.5768-5-robh@kernel.org>
* Rob Herring <robh@kernel.org> [180508 07:01]:
> omap3-devkit8000-common.dtsi defines a graph connection for DVI, but
> then omap3-devkit8000-lcd-common.dtsi overrides that with a graph
> connection for the LCD as the same output signals are used. This
> leaves an incomplete graph as the TFP410 output has only half a
> connection. The result is the following warning:
>
> arch/arm/boot/dts/omap3-devkit8000-lcd70.dtb: Warning (graph_endpoint): /encoder0/ports/port@0/endpoint: graph connection to node '/ocp@68000000/dss@48050000/port/endpoint' is not bidirectional
>
> Fix this by defining multiple endpoints which is the correct way to show
> a 1 to many connection.
Applying into omap-for-v4.18/dt-fixes thanks.
Tony
^ permalink raw reply
* Re: [PATCH] clk: qcom: Add some missing gcc clks for msm8996
From: Stephen Boyd @ 2018-05-15 18:05 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette
Cc: Rajendra Nayak, Nicolas Dechesne, Andy Gross, linux-arm-msm,
linux-soc, linux-clk, linux-kernel, devicetree
In-Reply-To: <20180418000926.4998-1-bjorn.andersson@linaro.org>
Quoting Bjorn Andersson (2018-04-17 17:09:26)
> @@ -3350,6 +3522,7 @@ static struct clk_regmap *gcc_msm8996_clocks[] = {
> [GCC_AGGRE0_CNOC_AHB_CLK] = &gcc_aggre0_cnoc_ahb_clk.clkr,
> [GCC_SMMU_AGGRE0_AXI_CLK] = &gcc_smmu_aggre0_axi_clk.clkr,
> [GCC_SMMU_AGGRE0_AHB_CLK] = &gcc_smmu_aggre0_ahb_clk.clkr,
> + [GCC_AGGRE1_PNOC_AHB_CLK] = &gcc_aggre1_pnoc_ahb_clk.clkr,
> [GCC_AGGRE2_UFS_AXI_CLK] = &gcc_aggre2_ufs_axi_clk.clkr,
> [GCC_AGGRE2_USB3_AXI_CLK] = &gcc_aggre2_usb3_axi_clk.clkr,
> [GCC_QSPI_AHB_CLK] = &gcc_qspi_ahb_clk.clkr,
> @@ -3360,6 +3533,19 @@ static struct clk_regmap *gcc_msm8996_clocks[] = {
> [GCC_PCIE_CLKREF_CLK] = &gcc_pcie_clkref_clk.clkr,
> [GCC_RX2_USB2_CLKREF_CLK] = &gcc_rx2_usb2_clkref_clk.clkr,
> [GCC_RX1_USB2_CLKREF_CLK] = &gcc_rx1_usb2_clkref_clk.clkr,
> + [GCC_HLOS1_VOTE_LPASS_CORE_SMMU_CLK] =
> + &hlos1_vote_lpass_core_smmu_clk.clkr,
drivers/clk/qcom/gcc-msm8996.c:3512:10: warning: Initializer entry defined twice
drivers/clk/qcom/gcc-msm8996.c:3536:10: also defined here
Resend please, or at least let me know what to do here. It looks like
the one you sent has the parent chain setup properly so that the bus
rcgs are in the chain of clks, while the one that is already there
doesn't have the chain setup.
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: exynos: Add support for audio over HDMI for Odroid X/X2/U3
From: Krzysztof Kozlowski @ 2018-05-15 17:39 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: robh+dt, devicetree, linux-samsung-soc, linux-arm-kernel,
linux-kernel, b.zolnierkie, m.szyprowski
In-Reply-To: <20180514080148.10150-1-s.nawrocki@samsung.com>
On Mon, May 14, 2018 at 10:01:48AM +0200, Sylwester Nawrocki wrote:
> This patch switches Odroid X/X2/U3 to use dedicated Odroid audio subsystem
> DT bindings instead of the simple-card in order to add support for audio
> over HDMI.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 33 +++++++++++++++----------
> arch/arm/boot/dts/exynos4412-odroidu3.dts | 6 ++---
> arch/arm/boot/dts/exynos4412-odroidx.dts | 6 ++---
> 3 files changed, 26 insertions(+), 19 deletions(-)
>
Thanks, applied.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/3] ARM: dra762: hwmod: Add MCAN support
From: Tony Lindgren @ 2018-05-15 17:23 UTC (permalink / raw)
To: Faiz Abbas
Cc: linux-kernel, linux-omap, devicetree, bcousson, robh+dt,
mark.rutland, paul, lokeshvutla, linux
In-Reply-To: <20180515171808.GR98604@atomide.com>
* Tony Lindgren <tony@atomide.com> [180515 17:20]:
> * Tony Lindgren <tony@atomide.com> [180515 17:17]:
> > * Faiz Abbas <faiz_abbas@ti.com> [180515 06:05]:
> > > Hi Tony,
> > >
> > > On Tuesday 01 May 2018 08:56 PM, Tony Lindgren wrote:
> > > > Hi,
> > > >
> > > > * Faiz Abbas <faiz_abbas@ti.com> [180408 09:59]:
> > > >> From: Lokesh Vutla <lokeshvutla@ti.com>
> > > >>
> > > >> Add MCAN hwmod data and register it for dra762 silicons.
> > > >>
> > > >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> > > >> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> > > >> ---
> > > >> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 +++++++++++++++++++++++++++++++
> > > >> 1 file changed, 32 insertions(+)
> > > >>
> > > >> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > > >> index 62352d1..a2cd7f8 100644
> > > >> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > > >> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > > >> @@ -1356,6 +1356,29 @@ static struct omap_hwmod dra7xx_mailbox13_hwmod = {
> > > >> };
> > > >>
> > > >> /*
> > > >> + * 'mcan' class
> > > >> + *
> > > >> + */
> > > >> +static struct omap_hwmod_class dra76x_mcan_hwmod_class = {
> > > >> + .name = "mcan",
> > > >> +};
> > > >
> > > > Looks like you're missing the related struct omap_hwmod_class_sysconfig
> > > > entry for this with the rev and sysconfig registers.
> > > >
> > >
> > > Sorry, I missed this email earlier. The MCAN module doesn't have
> > > sysconfig registers.
> >
> > If there's no sysconfig register you should have no need for adding
> > anything to omap_hwmod_7xx_data.c. We now have the dts clkctrl
> > clocks, you can just add that to the dts node for the device.
>
> However.. My guess is there is somewhere a top level interconnect
> target module that uses DRA7XX_CM_WKUPAON_ADC_CLKCTRL. And that
> module may have multiple child devices.
And I'm guessing the top level module with sysconfig related
registers is named "adc" :)
Regards,
Tony
^ permalink raw reply
* Re: [PATCH V3] Input: pm8941-pwrkey: add resin key capabilities
From: Dmitry Torokhov @ 2018-05-15 17:20 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Tirupathi Reddy, robh+dt, mark.rutland, linux-input,
linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20180507184659.GG2259@tuxbook-pro>
On Mon, May 07, 2018 at 11:46:59AM -0700, Bjorn Andersson wrote:
> On Fri 04 May 17:10 PDT 2018, Dmitry Torokhov wrote:
>
> > Hi Tirupathi,
> >
> > On Fri, Mar 23, 2018 at 11:53:12AM +0530, Tirupathi Reddy wrote:
> > > Add resin key support to handle different types of key events
> > > defined in different platforms.
> > >
> > > Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
> > > ---
> > > .../bindings/input/qcom,pm8941-pwrkey.txt | 32 +++++++++
> > > drivers/input/misc/pm8941-pwrkey.c | 81 ++++++++++++++++++++++
> > > 2 files changed, 113 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> [..]
> > > EXAMPLE
> > >
> > > pwrkey@800 {
> > > @@ -40,4 +66,10 @@ EXAMPLE
> > > interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
> > > debounce = <15625>;
> > > bias-pull-up;
> > > +
> > > + resin {
> > > + interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> > > + linux,code = <KEY_VOLUMEDOWN>;
> > > + bias-pull-up;
> > > + };
> > > };
> >
> > The new key and power key bindings are very similar, I would prefer if
> > we shared the parsing code and our new DTS looked like:
> >
> > power {
> > ...
> > };
> >
> > resin {
> > ...
> > };
> >
> > (we can easily keep backward compatibility with power properties being
> > in device node).
> >
>
> As discussed here https://patchwork.kernel.org/patch/9751627/ the PON
> block does, in addition to providing power and resin key support also
> handle the restart reason ("reboot bootloader" in Android).
>
> My interpretation of our conclusion was to come up with a new binding
> for the "pon" and a driver for this block that instantiates the power
> key device.
>
> It seems reasonable for such binding to describe the two keys, as you
> propose here Dmitry, and instantiates the two input devices based on
> this.
OK, I'll wait for the new driver and binding then.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/3] ARM: dra762: hwmod: Add MCAN support
From: Tony Lindgren @ 2018-05-15 17:18 UTC (permalink / raw)
To: Faiz Abbas
Cc: linux-kernel, linux-omap, devicetree, bcousson, robh+dt,
mark.rutland, paul, lokeshvutla, linux
In-Reply-To: <20180515171459.GP98604@atomide.com>
* Tony Lindgren <tony@atomide.com> [180515 17:17]:
> * Faiz Abbas <faiz_abbas@ti.com> [180515 06:05]:
> > Hi Tony,
> >
> > On Tuesday 01 May 2018 08:56 PM, Tony Lindgren wrote:
> > > Hi,
> > >
> > > * Faiz Abbas <faiz_abbas@ti.com> [180408 09:59]:
> > >> From: Lokesh Vutla <lokeshvutla@ti.com>
> > >>
> > >> Add MCAN hwmod data and register it for dra762 silicons.
> > >>
> > >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> > >> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> > >> ---
> > >> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 +++++++++++++++++++++++++++++++
> > >> 1 file changed, 32 insertions(+)
> > >>
> > >> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > >> index 62352d1..a2cd7f8 100644
> > >> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > >> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> > >> @@ -1356,6 +1356,29 @@ static struct omap_hwmod dra7xx_mailbox13_hwmod = {
> > >> };
> > >>
> > >> /*
> > >> + * 'mcan' class
> > >> + *
> > >> + */
> > >> +static struct omap_hwmod_class dra76x_mcan_hwmod_class = {
> > >> + .name = "mcan",
> > >> +};
> > >
> > > Looks like you're missing the related struct omap_hwmod_class_sysconfig
> > > entry for this with the rev and sysconfig registers.
> > >
> >
> > Sorry, I missed this email earlier. The MCAN module doesn't have
> > sysconfig registers.
>
> If there's no sysconfig register you should have no need for adding
> anything to omap_hwmod_7xx_data.c. We now have the dts clkctrl
> clocks, you can just add that to the dts node for the device.
However.. My guess is there is somewhere a top level interconnect
target module that uses DRA7XX_CM_WKUPAON_ADC_CLKCTRL. And that
module may have multiple child devices.
Regards,
Tony
^ 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