* 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 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Dan Murphy @ 2018-05-15 21:50 UTC (permalink / raw)
To: Jacek Anaszewski, robh+dt, mark.rutland, pavel
Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <b0f78b38-52af-aa40-f838-217d7721157a@gmail.com>
Jacek
On 05/15/2018 04:24 PM, Jacek Anaszewski wrote:
> 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.
OK not sure how this is working then.
>
>> +};
>> +
>> +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.
I will look into it.
>
>> + 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.
See below
>
>> +
>> + 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?
I did actually follow the same approach here as the as3645a driver.
I used this driver since I have a data sheet and can read the code.
This is a copy, paste, slight modify from that as3645a driver as3645a_led_class_setup function.
I used the devm register function above as a variation.
I will have to dig further into it.
>
> 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.
I will look at it again
>
>> + 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".
Hmm. I remember this conversation from my other drivers.
I just picked the wrong dt node name.
I will fix it.
>
>> + 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.
Ack
>
>> +
>> + 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.
Strobe can be used as a noun to define an electronic camera flash. But this is
really a US English definition.
I can change it to flash
>
>> +
>> + 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.
OK.
>
>> +
>> + 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");
>>
>
--
------------------
Dan Murphy
^ permalink raw reply
* Re: [PATCH v6 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Andy Shevchenko @ 2018-05-15 21:56 UTC (permalink / raw)
To: Dan Murphy
Cc: Rob Herring, Mark Rutland, Jacek Anaszewski, Pavel Machek,
devicetree, Linux Kernel Mailing List, Linux LED Subsystem
In-Reply-To: <20180515154352.20263-2-dmurphy@ti.com>
On Tue, May 15, 2018 at 6:43 PM, Dan Murphy <dmurphy@ti.com> 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
> + depends on LEDS_CLASS && I2C && OF
What is OF specific in this driver?
> +#define LM3601X_STRB_LVL_TRIG ~BIT(3)
> +#define LM36010_BOOST_LIMIT_19 ~BIT(5)
> +#define LM36010_BOOST_FREQ_2MHZ ~BIT(6)
> +#define LM36010_BOOST_MODE_NORM ~BIT(7)
These looks rather strange. Shouldn't be just 0:s?
> +struct lm3601x_led {
> + struct mutex lock;
> + struct regmap *regmap;
> + struct i2c_client *client;
> + struct device_node *led_node;
Why do you need this?
> +};
> +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 },
Huh?!
strobe_timeout = (x + 1) * 40 * MSECS_IN_SEC;
> +};
> + 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;
Better to put lines here in reversed xmas tree order (longest first).
> + reg_count = ARRAY_SIZE(strobe_timeouts);
> + for (i = 0; i < reg_count; i++) {
> + if (led->strobe_timeout > strobe_timeouts[i].timeout)
> + continue;
binary search?
Check lib/bsearch.c (IIRC).
But! See above the formula.
> + brightness_val = (brightness/2);
Spaces.
> +static int lm3601x_register_leds(struct lm3601x_led *led)
> +{
> + int err = -ENODEV;
Useless assignment.
> + err = led_classdev_flash_register(&led->client->dev,
> + fled_cdev);
> +
> + return err;
This is return led_...();
> +}
> + ret = of_property_read_string(led->led_node, "label", &name);
device_property_...();
> + if (!ret)
if (ret) sounds more natural. And better just to split
> + 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:torch", led->led_node->name);
const char *tmp;
ret = device_property_read_...(&tmp);
if (ret)
tmp = ...
sprintf(...);
> + led = devm_kzalloc(&client->dev,
> + sizeof(struct lm3601x_led), GFP_KERNEL);
sizeof(*led) and one line in the result
> +static const struct i2c_device_id lm3601x_id[] = {
> + { "LM36010", CHIP_LM36010 },
> + { "LM36011", CHIP_LM36011 },
> + { },
Terminators better w/o comma.
> +};
> +MODULE_DEVICE_TABLE(i2c, lm3601x_id);
> +
> +static const struct of_device_id of_lm3601x_leds_match[] = {
> + { .compatible = "ti,lm36010", },
> + { .compatible = "ti,lm36011", },
> + {},
Ditto.
> +};
> +MODULE_DEVICE_TABLE(of, of_lm3601x_leds_match);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH net-next 0/2] of: mdio: Fall back to mdiobus_register() with np is NULL
From: Florian Fainelli @ 2018-05-15 21:59 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan
Hi all,
This patch series updates of_mdiobus_register() such that when the device_node
argument is NULL, it calls mdiobus_register() directly. This is consistent with
the behavior of of_mdiobus_register() when CONFIG_OF=n.
I only converted the most obvious drivers, there are others that have a much
less obvious behavior and specifically attempt to deal with CONFIG_ACPI.
Florian Fainelli (2):
of: mdio: Fall back to mdiobus_register() with np is NULL
drivers: net: Remove device_node checks with of_mdiobus_register()
drivers/net/dsa/bcm_sf2.c | 8 ++------
drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
drivers/net/ethernet/freescale/fec_main.c | 8 ++------
drivers/net/ethernet/marvell/mvmdio.c | 5 +----
drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
drivers/net/ethernet/ti/davinci_mdio.c | 6 ++----
drivers/net/phy/mdio-gpio.c | 6 +-----
drivers/net/phy/mdio-mscc-miim.c | 6 +-----
drivers/net/usb/lan78xx.c | 7 ++-----
drivers/of/of_mdio.c | 6 ++++++
12 files changed, 25 insertions(+), 60 deletions(-)
--
2.14.1
^ permalink raw reply
* [PATCH net-next 1/2] of: mdio: Fall back to mdiobus_register() with np is NULL
From: Florian Fainelli @ 2018-05-15 21:59 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan
In-Reply-To: <20180515215930.12115-1-f.fainelli@gmail.com>
When the Device Tree node specified is NULL, fall back to
mdiobus_register(). We have a number of drivers having a similar pattern
which is:
if (np)
of_mdiobus_register()
else
mdiobus_register()
so incorporate that behavior within the core of_mdiobus_register()
function. This is also consistent with the stub version that we defined
when CONFIG_OF=n.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/of/of_mdio.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 8c0c92712fc9..2341dbf675bf 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -204,6 +204,12 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
bool scanphys = false;
int addr, rc;
+ /* If the Device Tree node does not exist, fall back to traditional
+ * registration
+ */
+ if (!np)
+ return mdiobus_register(mdio);
+
/* Do not continue if the node is disabled */
if (!of_device_is_available(np))
return -ENODEV;
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 2/2] drivers: net: Remove device_node checks with of_mdiobus_register()
From: Florian Fainelli @ 2018-05-15 21:59 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan
In-Reply-To: <20180515215930.12115-1-f.fainelli@gmail.com>
A number of drivers have the following pattern:
if (np)
of_mdiobus_register()
else
mdiobus_register()
which the implementation of of_mdiobus_register() now takes care of.
Remove that pattern in drivers that strictly adhere to it.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 8 ++------
drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
drivers/net/ethernet/freescale/fec_main.c | 8 ++------
drivers/net/ethernet/marvell/mvmdio.c | 5 +----
drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
drivers/net/ethernet/ti/davinci_mdio.c | 6 ++----
drivers/net/phy/mdio-gpio.c | 6 +-----
drivers/net/phy/mdio-mscc-miim.c | 6 +-----
drivers/net/usb/lan78xx.c | 7 ++-----
11 files changed, 19 insertions(+), 60 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index ac621f44237a..02e8982519ce 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -450,12 +450,8 @@ static int bcm_sf2_mdio_register(struct dsa_switch *ds)
priv->slave_mii_bus->parent = ds->dev->parent;
priv->slave_mii_bus->phy_mask = ~priv->indir_phy_mask;
- if (dn)
- err = of_mdiobus_register(priv->slave_mii_bus, dn);
- else
- err = mdiobus_register(priv->slave_mii_bus);
-
- if (err)
+ err = of_mdiobus_register(priv->slave_mii_bus, dn);
+ if (err && dn)
of_node_put(dn);
return err;
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b23c11d9f4b2..2bb3f03ee1cb 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2454,10 +2454,7 @@ static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
return err;
}
- if (np)
- err = of_mdiobus_register(bus, np);
- else
- err = mdiobus_register(bus);
+ err = of_mdiobus_register(bus, np);
if (err) {
dev_err(chip->dev, "Cannot register MDIO bus (%d)\n", err);
mv88e6xxx_g2_irq_mdio_free(chip, bus);
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b4c9268100bb..3e93df5d4e3b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -591,16 +591,10 @@ static int macb_mii_init(struct macb *bp)
dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
np = bp->pdev->dev.of_node;
+ if (pdata)
+ bp->mii_bus->phy_mask = pdata->phy_mask;
- if (np) {
- err = of_mdiobus_register(bp->mii_bus, np);
- } else {
- if (pdata)
- bp->mii_bus->phy_mask = pdata->phy_mask;
-
- err = mdiobus_register(bp->mii_bus);
- }
-
+ err = of_mdiobus_register(bp->mii_bus, np);
if (err)
goto err_out_free_mdiobus;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d4604bc8eb5b..f3e43db0d6cb 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2052,13 +2052,9 @@ static int fec_enet_mii_init(struct platform_device *pdev)
fep->mii_bus->parent = &pdev->dev;
node = of_get_child_by_name(pdev->dev.of_node, "mdio");
- if (node) {
- err = of_mdiobus_register(fep->mii_bus, node);
+ err = of_mdiobus_register(fep->mii_bus, node);
+ if (node)
of_node_put(node);
- } else {
- err = mdiobus_register(fep->mii_bus);
- }
-
if (err)
goto err_out_free_mdiobus;
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 0495487f7b42..c5dac6bd2be4 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -348,10 +348,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
goto out_mdio;
}
- if (pdev->dev.of_node)
- ret = of_mdiobus_register(bus, pdev->dev.of_node);
- else
- ret = mdiobus_register(bus);
+ ret = of_mdiobus_register(bus, pdev->dev.of_node);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
goto out_mdio;
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 5970d9e5ddf1..8dd41e08a6c6 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3025,15 +3025,10 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
pdev->name, pdev->id);
/* register MDIO bus */
- if (dev->of_node) {
- ret = of_mdiobus_register(mdp->mii_bus, dev->of_node);
- } else {
- if (pd->phy_irq > 0)
- mdp->mii_bus->irq[pd->phy] = pd->phy_irq;
-
- ret = mdiobus_register(mdp->mii_bus);
- }
+ if (pd->phy_irq > 0)
+ mdp->mii_bus->irq[pd->phy] = pd->phy_irq;
+ ret = of_mdiobus_register(mdp->mii_bus, dev->of_node);
if (ret)
goto out_free_bus;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index f5f37bfa1d58..5df1a608e566 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -233,10 +233,7 @@ int stmmac_mdio_register(struct net_device *ndev)
new_bus->phy_mask = mdio_bus_data->phy_mask;
new_bus->parent = priv->device;
- if (mdio_node)
- err = of_mdiobus_register(new_bus, mdio_node);
- else
- err = mdiobus_register(new_bus);
+ err = of_mdiobus_register(new_bus, mdio_node);
if (err != 0) {
dev_err(dev, "Cannot register the MDIO bus\n");
goto bus_register_fail;
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 98a1c97fb95e..e720244e7f71 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -431,10 +431,8 @@ static int davinci_mdio_probe(struct platform_device *pdev)
*/
if (dev->of_node && of_get_child_count(dev->of_node)) {
data->skip_scan = true;
- ret = of_mdiobus_register(data->bus, dev->of_node);
- } else {
- ret = mdiobus_register(data->bus);
- }
+
+ ret = of_mdiobus_register(data->bus, dev->of_node);
if (ret)
goto bail_out;
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index b501221819e1..4e4c8daf44c3 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -179,11 +179,7 @@ static int mdio_gpio_probe(struct platform_device *pdev)
if (!new_bus)
return -ENODEV;
- if (pdev->dev.of_node)
- ret = of_mdiobus_register(new_bus, pdev->dev.of_node);
- else
- ret = mdiobus_register(new_bus);
-
+ ret = of_mdiobus_register(new_bus, pdev->dev.of_node);
if (ret)
mdio_gpio_bus_deinit(&pdev->dev);
diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c
index 8c689ccfdbca..badbc99bedd3 100644
--- a/drivers/net/phy/mdio-mscc-miim.c
+++ b/drivers/net/phy/mdio-mscc-miim.c
@@ -151,11 +151,7 @@ static int mscc_miim_probe(struct platform_device *pdev)
}
}
- if (pdev->dev.of_node)
- ret = of_mdiobus_register(bus, pdev->dev.of_node);
- else
- ret = mdiobus_register(bus);
-
+ ret = of_mdiobus_register(bus, pdev->dev.of_node);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
return ret;
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 91761436709a..8dff87ec6d99 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1843,12 +1843,9 @@ static int lan78xx_mdio_init(struct lan78xx_net *dev)
}
node = of_get_child_by_name(dev->udev->dev.of_node, "mdio");
- if (node) {
- ret = of_mdiobus_register(dev->mdiobus, node);
+ ret = of_mdiobus_register(dev->mdiobus, node);
+ if (node)
of_node_put(node);
- } else {
- ret = mdiobus_register(dev->mdiobus);
- }
if (ret) {
netdev_err(dev->net, "can't register MDIO bus\n");
goto exit1;
--
2.14.1
^ permalink raw reply related
* Re: [PATCH] clk:aspeed: Fix reset bits for PCI/VGA and PECI
From: Stephen Boyd @ 2018-05-15 22:02 UTC (permalink / raw)
To: Jae Hyun Yoo, Rob Herring
Cc: Michael Turquette, Joel Stanley, Andrew Jeffery, Mark Rutland,
linux-clk, linux-arm-kernel, linux-aspeed, linux-kernel,
devicetree, stable
In-Reply-To: <5a449d93-eef0-4243-9889-bd13786edde1@linux.intel.com>
Quoting Jae Hyun Yoo (2018-05-01 09:27:32)
> On 5/1/2018 8:02 AM, Rob Herring wrote:
> > On Thu, Apr 26, 2018 at 10:22:32AM -0700, Jae Hyun Yoo wrote:
> >> diff --git a/include/dt-bindings/clock/aspeed-clock.h b/include/dt-bindings/clock/aspeed-clock.h
> >> index d3558d897a4d..8d69b9134bef 100644
> >> --- a/include/dt-bindings/clock/aspeed-clock.h
> >> +++ b/include/dt-bindings/clock/aspeed-clock.h
> >> @@ -45,7 +45,7 @@
> >> #define ASPEED_RESET_JTAG_MASTER 3
> >> #define ASPEED_RESET_MIC 4
> >> #define ASPEED_RESET_PWM 5
> >> -#define ASPEED_RESET_PCIVGA 6
> >> +#define ASPEED_RESET_PECI 6
> >
> > You can't really be changing these as they represent an ABI.
> >
> > Is there no PCIVGA reset?
> >
>
> This is a bug fixing. Previously, PCI/VGA used PECI reset bit so this
> patch corrects the reset bit for PCI/VGA from bit '10' to bit '8', and
> it adds PECI reset bit '10' here as it can't be combined with a clock
> gate bit.
>
Presumably nobody is using the #define because it's wrong, so this is OK
for me. I'll apply to clk-next and yank it if Rob objects.
^ permalink raw reply
* Re: [PATCH v6 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Dan Murphy @ 2018-05-15 22:08 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rob Herring, Mark Rutland, Jacek Anaszewski, Pavel Machek,
devicetree, Linux Kernel Mailing List, Linux LED Subsystem
In-Reply-To: <CAHp75VegjkiiD1qTGpi9rr6cg=72t9kSwDrG=sUpUWFVO58M4A@mail.gmail.com>
Andy
Thanks for the review
On 05/15/2018 04:56 PM, Andy Shevchenko wrote:
> On Tue, May 15, 2018 at 6:43 PM, Dan Murphy <dmurphy@ti.com> 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
>
>> + depends on LEDS_CLASS && I2C && OF
>
> What is OF specific in this driver?
as3645a_led_class_setup has a "of" dependency
>
>
>> +#define LM3601X_STRB_LVL_TRIG ~BIT(3)
>
>> +#define LM36010_BOOST_LIMIT_19 ~BIT(5)
>
>> +#define LM36010_BOOST_FREQ_2MHZ ~BIT(6)
>
>> +#define LM36010_BOOST_MODE_NORM ~BIT(7)
>
> These looks rather strange. Shouldn't be just 0:s?
I don't actually use these so I can just remove them
>
>> +struct lm3601x_led {
>> + struct mutex lock;
>> + struct regmap *regmap;
>> + struct i2c_client *client;
>
>> + struct device_node *led_node;
>
> Why do you need this?
I don't but I can store it locally. I was using the nodes in the register
functions in previous versions.
>
>> +};
>
>> +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 },
>
> Huh?!
Please give comments that actually mean something other wise I will opt to ignore them.
>
> strobe_timeout = (x + 1) * 40 * MSECS_IN_SEC;
Not sure what equation you are trying to point out here. But if you are trying to apply
a timeout step you cannot do this with this part. As pointed out in the DT doc the timeout
step is not linear.
>
>> +};
>
>> + 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;
>
> Better to put lines here in reversed xmas tree order (longest first).
I can do that, again this is a preference.
>
>> + reg_count = ARRAY_SIZE(strobe_timeouts);
>> + for (i = 0; i < reg_count; i++) {
>> + if (led->strobe_timeout > strobe_timeouts[i].timeout)
>> + continue;
>
> binary search?
>
> Check lib/bsearch.c (IIRC).
>
> But! See above the formula.
>
>> + brightness_val = (brightness/2);
>
> Spaces.
Not sure what this means checkpatch was clean
>
>> +static int lm3601x_register_leds(struct lm3601x_led *led)
>> +{
>
>
>> + int err = -ENODEV;
>
> Useless assignment.
This is an artifact from prior versions I can remove it
>
>> + err = led_classdev_flash_register(&led->client->dev,
>> + fled_cdev);
>> +
>> + return err;
>
> This is return led_...();
That is a preference. It does not have to be that way.
>
>> +}
>
>> + ret = of_property_read_string(led->led_node, "label", &name);
>
> device_property_...();
It can be if the maintainer is requesting this.
Is the trend to move to these functions?
Most drivers use the "of" calls.
>
>> + if (!ret)
>
> if (ret) sounds more natural. And better just to split
>
>> + 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:torch", led->led_node->name);
>
> const char *tmp;
>
> ret = device_property_read_...(&tmp);
> if (ret)
> tmp = ...
> sprintf(...);
>
>> + led = devm_kzalloc(&client->dev,
>> + sizeof(struct lm3601x_led), GFP_KERNEL);
>
> sizeof(*led) and one line in the result
>
>> +static const struct i2c_device_id lm3601x_id[] = {
>> + { "LM36010", CHIP_LM36010 },
>> + { "LM36011", CHIP_LM36011 },
>> + { },
>
> Terminators better w/o comma.
Looking at other drivers adding comma's on the sentinel is accepted. See the as3645a driver
>
>> +};
>> +MODULE_DEVICE_TABLE(i2c, lm3601x_id);
>> +
>> +static const struct of_device_id of_lm3601x_leds_match[] = {
>> + { .compatible = "ti,lm36010", },
>> + { .compatible = "ti,lm36011", },
>> + {},
>
> Ditto.
See above
>
>> +};
>> +MODULE_DEVICE_TABLE(of, of_lm3601x_leds_match);
>
>
>
>
--
------------------
Dan Murphy
^ permalink raw reply
* Re: [PATCH v1 1/1] clk: npcm750: fix base address and of_clk_get_by_name error handling. Also update error messages to be more informative
From: Stephen Boyd @ 2018-05-15 22:15 UTC (permalink / raw)
To: avifishman70, brendanhiggins, linux, mark.rutland, raltherr,
robh+dt, tmaimon77, weiyongjunl
Cc: devicetree, openbmc, linux-kernel, Tali Perry, Wei Yongjun,
linux-arm-kernel
In-Reply-To: <1525159407-31708-2-git-send-email-tali.perry1@gmail.com>
Quoting Tali Perry (2018-05-01 00:23:27)
>
> Nuvoton NPCM7XX Clock Controller
> fix base address and of_clk_get_by_name error handling.
> Also update error messages to be more informative.
>
> In case clk_base allocation is erronoeous the return value is null.
> Also fix handling of of_clk_get_by_name returns an error.
> Print a better error message pointing to the dt-binding documention.
>
>
> Signed-off-by: Tali Perry <tali.perry1@gmail.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
>
> Tali Perry (2):
>
This doesn't apply to clk-next so I'm not sure what's going on. Please
resend if something needs to be fixed.
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: clock: mediatek: add g3dsys bindings
From: Stephen Boyd @ 2018-05-15 22:22 UTC (permalink / raw)
To: mark.rutland, matthias.bgg, mturquette, p.zabel, robh+dt, sboyd
Cc: devicetree, linux-mediatek, linux-clk, linux-arm-kernel,
linux-kernel, Sean Wang
In-Reply-To: <665c38d5803573aa9a01471253f406301b1123a1.1524816502.git.sean.wang@mediatek.com>
Quoting sean.wang@mediatek.com (2018-04-27 01:14:43)
> From: Sean Wang <sean.wang@mediatek.com>
>
> Add bindings to g3dsys providing necessary clock and reset control to
> Mali-450.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v2 3/6] dt-bindings: clock: mediatek: add entry for Mali-450 node to refer
From: Stephen Boyd @ 2018-05-15 22:22 UTC (permalink / raw)
To: mark.rutland, matthias.bgg, mturquette, p.zabel, robh+dt, sboyd
Cc: devicetree, linux-mediatek, linux-clk, linux-arm-kernel,
linux-kernel, Sean Wang
In-Reply-To: <082c3040e2be27d30e0642943f6df35ff4de5666.1524816502.git.sean.wang@mediatek.com>
Quoting sean.wang@mediatek.com (2018-04-27 01:14:44)
> From: Sean Wang <sean.wang@mediatek.com>
>
> Just add binding for a required clock referenced by Mali-450 on MT7623
> or MT2701 SoC.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v2 4/6] dt-bindings: reset: mediatek: add entry for Mali-450 node to refer
From: Stephen Boyd @ 2018-05-15 22:22 UTC (permalink / raw)
To: mark.rutland, matthias.bgg, mturquette, p.zabel, robh+dt, sboyd
Cc: devicetree, Sean Wang, linux-kernel, linux-mediatek, linux-clk,
linux-arm-kernel
In-Reply-To: <d9f327fa30c987ac9cf3a349551bbaa037d1b24a.1524816502.git.sean.wang@mediatek.com>
Quoting sean.wang@mediatek.com (2018-04-27 01:14:45)
> From: Sean Wang <sean.wang@mediatek.com>
>
> Just add binding for a required reset referenced by Mali-450 on MT7623
> or MT2701 SoC.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v2 5/6] clk: mediatek: add g3dsys support for MT2701 and MT7623
From: Stephen Boyd @ 2018-05-15 22:22 UTC (permalink / raw)
To: mark.rutland, matthias.bgg, mturquette, p.zabel, robh+dt, sboyd
Cc: devicetree, linux-mediatek, linux-clk, linux-arm-kernel,
linux-kernel, Sean Wang
In-Reply-To: <caf1cbbd0758cbb9c25c081f4d8fdb9f64769a44.1524816502.git.sean.wang@mediatek.com>
Quoting sean.wang@mediatek.com (2018-04-27 01:14:46)
> From: Sean Wang <sean.wang@mediatek.com>
>
> Add clock driver support for g3dsys on MT2701 and MT7623, which is
> providing essential clock gate and reset controller to Mali-450.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v6 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Andy Shevchenko @ 2018-05-15 22:24 UTC (permalink / raw)
To: Dan Murphy
Cc: Rob Herring, Mark Rutland, Jacek Anaszewski, Pavel Machek,
devicetree, Linux Kernel Mailing List, Linux LED Subsystem
In-Reply-To: <08dda10d-f865-9cf2-b9a5-c79cbab5da98@ti.com>
On Wed, May 16, 2018 at 1:08 AM, Dan Murphy <dmurphy@ti.com> wrote:
> On 05/15/2018 04:56 PM, Andy Shevchenko wrote:
>> On Tue, May 15, 2018 at 6:43 PM, Dan Murphy <dmurphy@ti.com> wrote:
>>> + depends on LEDS_CLASS && I2C && OF
>>
>> What is OF specific in this driver?
>
> as3645a_led_class_setup has a "of" dependency
So what? Is it called from this driver or?
>>> +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 },
>>
>> Huh?!
>
> Please give comments that actually mean something other wise I will opt to ignore them.
I did below.
>> strobe_timeout = (x + 1) * 40 * MSECS_IN_SEC;
>
> Not sure what equation you are trying to point out here. But if you are trying to apply
> a timeout step you cannot do this with this part. As pointed out in the DT doc the timeout
> step is not linear.
Yeah, I know people are more than often too lazy to think.
if (x < 9)
strobe_timeout = (x + 1) * 40 * MSECS_IN_SEC;
else
strobe_timeout = (400 + (x - 9) * 200) * MSECS_IN_SEC;
>>> + brightness_val = (brightness/2);
>>
>> Spaces.
>
> Not sure what this means checkpatch was clean
Even besides missed whispaces it has redundant parens.
checkpatch is not a silver bullet to get your code clean and nice.
>> This is return led_...();
>
> That is a preference. It does not have to be that way.
What do you mean? We do not appreciate +LOCs for no (or even nagative!) benefit.
>>> + ret = of_property_read_string(led->led_node, "label", &name);
>>
>> device_property_...();
>
> It can be if the maintainer is requesting this.
Jacek, if you need rationale behind this comment it's here: the driver
has nothing DT specific and getting rid of OF centric programming
allows to reuse the driver on non-DT platforms w/o touching a source
code.
> Is the trend to move to these functions?
See above.
> Most drivers use the "of" calls.
So what?
>>> + if (!ret)
>>
>> if (ret) sounds more natural. And better just to split
>>
>>> + 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:torch", led->led_node->name);
>>
>> const char *tmp;
>>
>> ret = device_property_read_...(&tmp);
>> if (ret)
>> tmp = ...
>> sprintf(...);
No comments on this?
>>> + led = devm_kzalloc(&client->dev,
>>> + sizeof(struct lm3601x_led), GFP_KERNEL);
>>
>> sizeof(*led) and one line in the result
And this?
>>> + { },
>>
>> Terminators better w/o comma.
>
> Looking at other drivers adding comma's on the sentinel is accepted. See the as3645a driver
So what?
Terminator at compile time even better.
>>> + {},
>>
>> Ditto.
>
> See above
See above.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCHv3] arm64: dts: stratix10: Add QSPI support for Stratix10
From: thor.thayer @ 2018-05-15 22:26 UTC (permalink / raw)
To: dinguyen, robh+dt, mark.rutland
Cc: catalin.marinas, will.deacon, devicetree, linux-arm-kernel,
linux-kernel, Thor Thayer
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
v3 remove unused bus-num node
use device id from table (n25q00a)
---
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 21 +++++++++++++
.../boot/dts/altera/socfpga_stratix10_socdk.dts | 35 ++++++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index e6b059378dc0..d8c94d5ff4b4 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -119,6 +119,12 @@
#clock-cells = <0>;
compatible = "fixed-clock";
};
+
+ qspi_clk: qspi-clk {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <200000000>;
+ };
};
gmac0: ethernet@ff800000 {
@@ -466,5 +472,20 @@
interrupts = <16 4>, <48 4>;
};
};
+
+ 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>;
+
+ status = "disabled";
+ };
};
};
diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
index f9b1ef12db48..6edc4fa9fd42 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
@@ -147,3 +147,38 @@
reg = <0x68>;
};
};
+
+&qspi {
+ flash@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "n25q00a";
+ reg = <0>;
+ spi-max-frequency = <50000000>;
+
+ m25p,fast-read;
+ cdns,page-size = <256>;
+ cdns,block-size = <16>;
+ cdns,read-delay = <1>;
+ cdns,tshsl-ns = <50>;
+ cdns,tsd2d-ns = <50>;
+ cdns,tchsh-ns = <4>;
+ cdns,tslch-ns = <4>;
+
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ qspi_boot: partition@0 {
+ label = "Boot and fpga data";
+ reg = <0x0 0x4000000>;
+ };
+
+ qspi_rootfs: partition@4000000 {
+ label = "Root Filesystem - JFFS2";
+ reg = <0x4000000 0x4000000>;
+ };
+ };
+ };
+};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v6 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Andy Shevchenko @ 2018-05-15 22:27 UTC (permalink / raw)
To: Dan Murphy
Cc: Rob Herring, Mark Rutland, Jacek Anaszewski, Pavel Machek,
devicetree, Linux Kernel Mailing List, Linux LED Subsystem
In-Reply-To: <CAHp75Vcz08zrs3bgMFcvYwRrN_A43D=Nvt5cFDQf8m9whB=yhA@mail.gmail.com>
On Wed, May 16, 2018 at 1:24 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, May 16, 2018 at 1:08 AM, Dan Murphy <dmurphy@ti.com> wrote:
>> On 05/15/2018 04:56 PM, Andy Shevchenko wrote:
>>>> +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 },
>>> strobe_timeout = (x + 1) * 40 * MSECS_IN_SEC;
>>
>> Not sure what equation you are trying to point out here. But if you are trying to apply
>> a timeout step you cannot do this with this part. As pointed out in the DT doc the timeout
>> step is not linear.
>
> Yeah, I know people are more than often too lazy to think.
>
> if (x < 9)
> strobe_timeout = (x + 1) * 40 * MSECS_IN_SEC;
> else
> strobe_timeout = (400 + (x - 9) * 200) * MSECS_IN_SEC;
Even just (x - 7) * 200 * ...
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v10 00/27] ARM: davinci: convert to common clock framework
From: Adam Ford @ 2018-05-15 22:44 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: David Lechner, linux-clk, devicetree, arm-soc, Michael Turquette,
Stephen Boyd, Rob Herring, Mark Rutland, Sekhar Nori,
Kevin Hilman, Bartosz Golaszewski, Linux Kernel Mailing List
In-Reply-To: <CAMRc=MdO0EtzDHp3RF4ym3umWM_KfhvpWHV2+Y3270ygKz9iGw@mail.gmail.com>
On Tue, May 15, 2018 at 4:25 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 2018-05-14 2:40 GMT+02:00 Adam Ford <aford173@gmail.com>:
>> On Wed, May 9, 2018 at 12:25 PM, David Lechner <david@lechnology.com> wrote:
>>> This series converts mach-davinci to use the common clock framework.
>>>
>>> The series works like this, the first 3 patches fix some issues with the clock
>>> drivers that have already been accepted into the mainline kernel.
>>>
>>> Then, starting with "ARM: davinci: pass clock as parameter to
>>> davinci_timer_init()", we get the mach code ready for the switch by adding the
>>> code needed for the new clock drivers and adding #ifndef CONFIG_COMMON_CLK
>>> around the legacy clocks so that we can switch easily between the old and the
>>> new.
>>>
>>> "ARM: davinci: switch to common clock framework" actually flips the switch
>>> to start using the new clock drivers. Then the next 8 patches remove all
>>> of the old clock code.
>>>
>>> The final four patches add device tree clock support to the one SoC that
>>> supports it.
>>>
>>> This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
>>> board file).
>>>
>>
>> I am not sure if I did something wrong, but I attempted to build and I
>> wasn't able to boot the da850-evm.dtb your repo common-clk-v11,
>> however the legacy board file boot was OK.
>>
>> make davinci_all_defconfig ARCH=arm
>> make zImage modules da850-evm.dtb ARCH=arm CROSS_COMPILE=arm-linux- -j8
>>
>> 3140416 bytes read in 1464 ms (2 MiB/s)
>> 20353 bytes read in 15 ms (1.3 MiB/s)
>> ## Flattened Device Tree blob at c0600000
>> Booting using the fdt blob at 0xc0600000
>> Loading Device Tree to c7e57000, end c7e5ef80 ... OK
>>
>> Starting kernel ...
>>
>> Uncompressing Linux... done, booting the kernel.
>>
>> (and hang)
>>
>> If you have some suggestions, I am try them as I get time.
>>
>> adam
>>
>
> Runs fine on da850-lcdk and dm365-evm. I'll test the da850-evm
> tomorrow when I'll have access to it.
I set the bootargs to: bootargs=console=ttyS2,115200n8
clk_ignore_unused root=/dev/mmcblk0p2 rw rootfstype=ext4 rootwait
I enabled DEBUG_LL and EARLY_PRINTK, yet when it loads, I only get:
## Flattened Device Tree blob at c0600000
Booting using the fdt blob at 0xc0600000
Loading Device Tree to c7e57000, end c7e5ef35 ... OK
Starting kernel ...
Uncompressing Linux... done, booting the kernel.
I am doing this at my home, so I don't have a debugger for the
DA850-EVM. I am using a SOM that is an AM1808, but I vaguely remember
something about enabling a DSP clock somewhere, but I cannot seem to
find the e-mail. I know its counter intuitive that we'd need to
enable a clock that runs the DSP since it doesn't exist on the AM1808,
but I would have thought the clk_ignore_unused would have worked
around that issue.
If someone else has a DA850-EVM or suggestions, I'm willing to try
them as I have time.
adam
>
> Bart
^ permalink raw reply
* Re: [PATCH net-next 2/2] drivers: net: Remove device_node checks with of_mdiobus_register()
From: Grygorii Strashko @ 2018-05-15 22:57 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, Nicolas Ferre,
Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan, Russell King, Geert Uytterhoeven,
Thomas
In-Reply-To: <20180515215930.12115-3-f.fainelli@gmail.com>
On 05/15/2018 04:59 PM, Florian Fainelli wrote:
> A number of drivers have the following pattern:
>
> if (np)
> of_mdiobus_register()
> else
> mdiobus_register()
>
> which the implementation of of_mdiobus_register() now takes care of.
> Remove that pattern in drivers that strictly adhere to it.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/dsa/bcm_sf2.c | 8 ++------
> drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
> drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
> drivers/net/ethernet/freescale/fec_main.c | 8 ++------
> drivers/net/ethernet/marvell/mvmdio.c | 5 +----
> drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
> drivers/net/ethernet/ti/davinci_mdio.c | 6 ++----
> drivers/net/phy/mdio-gpio.c | 6 +-----
> drivers/net/phy/mdio-mscc-miim.c | 6 +-----
> drivers/net/usb/lan78xx.c | 7 ++-----
> 11 files changed, 19 insertions(+), 60 deletions(-)
>
...
> goto bus_register_fail;
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index 98a1c97fb95e..e720244e7f71 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -431,10 +431,8 @@ static int davinci_mdio_probe(struct platform_device *pdev)
> */
> if (dev->of_node && of_get_child_count(dev->of_node)) {
It causes build error due to "{" above.
> data->skip_scan = true;
> - ret = of_mdiobus_register(data->bus, dev->of_node);
> - } else {
> - ret = mdiobus_register(data->bus);
> - }
> +
> + ret = of_mdiobus_register(data->bus, dev->of_node);
> if (ret)
> goto bail_out;
>
> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
> index b501221819e1..4e4c8daf44c3 100644
> --- a/drivers/net/phy/mdio-gpio.c
> +++ b/drivers/net/phy/mdio-gpio.c
> @@ -179,11 +179,7 @@ static int mdio_gpio_probe(struct platform_device *pdev)
[...]
--
regards,
-grygorii
^ permalink raw reply
* Re: [PATCH net-next 2/2] drivers: net: Remove device_node checks with of_mdiobus_register()
From: Florian Fainelli @ 2018-05-15 22:59 UTC (permalink / raw)
To: Grygorii Strashko, netdev
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, Nicolas Ferre,
Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan, Russell King, Geert Uytterhoeven,
Thomas
In-Reply-To: <1112dc66-5300-89f8-8382-ceb7a024e933@ti.com>
On 05/15/2018 03:57 PM, Grygorii Strashko wrote:
>
>
> On 05/15/2018 04:59 PM, Florian Fainelli wrote:
>> A number of drivers have the following pattern:
>>
>> if (np)
>> of_mdiobus_register()
>> else
>> mdiobus_register()
>>
>> which the implementation of of_mdiobus_register() now takes care of.
>> Remove that pattern in drivers that strictly adhere to it.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/net/dsa/bcm_sf2.c | 8 ++------
>> drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
>> drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
>> drivers/net/ethernet/freescale/fec_main.c | 8 ++------
>> drivers/net/ethernet/marvell/mvmdio.c | 5 +----
>> drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
>> drivers/net/ethernet/ti/davinci_mdio.c | 6 ++----
>> drivers/net/phy/mdio-gpio.c | 6 +-----
>> drivers/net/phy/mdio-mscc-miim.c | 6 +-----
>> drivers/net/usb/lan78xx.c | 7 ++-----
>> 11 files changed, 19 insertions(+), 60 deletions(-)
>>
>
> ...
>
>> goto bus_register_fail;
>> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
>> b/drivers/net/ethernet/ti/davinci_mdio.c
>> index 98a1c97fb95e..e720244e7f71 100644
>> --- a/drivers/net/ethernet/ti/davinci_mdio.c
>> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
>> @@ -431,10 +431,8 @@ static int davinci_mdio_probe(struct
>> platform_device *pdev)
>> */
>> if (dev->of_node && of_get_child_count(dev->of_node)) {
>
> It causes build error due to "{" above.
Humpf, shame on me for not enabling that driver, thanks for catching this!
>
>> data->skip_scan = true;
>> - ret = of_mdiobus_register(data->bus, dev->of_node);
>> - } else {
>> - ret = mdiobus_register(data->bus);
>> - }
>> +
>> + ret = of_mdiobus_register(data->bus, dev->of_node);
>> if (ret)
>> goto bail_out;
>> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
>> index b501221819e1..4e4c8daf44c3 100644
>> --- a/drivers/net/phy/mdio-gpio.c
>> +++ b/drivers/net/phy/mdio-gpio.c
>> @@ -179,11 +179,7 @@ static int mdio_gpio_probe(struct platform_device
>> *pdev)
>
> [...]
>
--
Florian
^ permalink raw reply
* Re: [PATCH 7/7] clk: meson: axg: add the audio clock controller driver
From: Stephen Boyd @ 2018-05-15 23:41 UTC (permalink / raw)
To: Carlo Caione, Jerome Brunet, Kevin Hilman, Neil Armstrong
Cc: Michael Turquette, linux-amlogic, linux-clk, devicetree,
linux-kernel
In-Reply-To: <82a486c6-4972-cb3d-5851-f020dfffb21e@baylibre.com>
Quoting Neil Armstrong (2018-04-26 01:49:29)
> On 25/04/2018 18:33, Jerome Brunet wrote:
> > +#endif /*__AXG_AUDIO_CLKC_H */
> >
>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Please trim replies.
^ permalink raw reply
* [PATCH net-next v2 0/2] of: mdio: Fall back to mdiobus_register() with NULL device_node
From: Florian Fainelli @ 2018-05-15 23:56 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan
Hi all,
This patch series updates of_mdiobus_register() such that when the device_node
argument is NULL, it calls mdiobus_register() directly. This is consistent with
the behavior of of_mdiobus_register() when CONFIG_OF=n.
I only converted the most obvious drivers, there are others that have a much
less obvious behavior and specifically attempt to deal with CONFIG_ACPI.
Changes in v2:
- fixed build error in davincin_mdio.c (Grygorii)
- reworked first patch a bit: commit message, subject and removed useless
code comment
Florian Fainelli (2):
of: mdio: Fall back to mdiobus_register() with NULL device_node
drivers: net: Remove device_node checks with of_mdiobus_register()
drivers/net/dsa/bcm_sf2.c | 8 ++------
drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
drivers/net/ethernet/freescale/fec_main.c | 8 ++------
drivers/net/ethernet/marvell/mvmdio.c | 5 +----
drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
drivers/net/ethernet/ti/davinci_mdio.c | 8 +++-----
drivers/net/phy/mdio-gpio.c | 6 +-----
drivers/net/phy/mdio-mscc-miim.c | 6 +-----
drivers/net/usb/lan78xx.c | 7 ++-----
drivers/of/of_mdio.c | 3 +++
12 files changed, 23 insertions(+), 61 deletions(-)
--
2.14.1
^ permalink raw reply
* [PATCH net-next v2 1/2] of: mdio: Fall back to mdiobus_register() with NULL device_node
From: Florian Fainelli @ 2018-05-15 23:56 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan
In-Reply-To: <20180515235619.27773-1-f.fainelli@gmail.com>
When the device_node specified is NULL, fall back to mdiobus_register().
We have a number of drivers having a similar pattern which is:
if (np)
of_mdiobus_register()
else
mdiobus_register()
so incorporate that behavior within the core of_mdiobus_register()
function. This is also consistent with the stub version that we defined
when CONFIG_OF=n.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/of/of_mdio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 8c0c92712fc9..d963baf8e53a 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -204,6 +204,9 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
bool scanphys = false;
int addr, rc;
+ if (!np)
+ return mdiobus_register(mdio);
+
/* Do not continue if the node is disabled */
if (!of_device_is_available(np))
return -ENODEV;
--
2.14.1
^ permalink raw reply related
* [PATCH net-next v2 2/2] drivers: net: Remove device_node checks with of_mdiobus_register()
From: Florian Fainelli @ 2018-05-15 23:56 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan
In-Reply-To: <20180515235619.27773-1-f.fainelli@gmail.com>
A number of drivers have the following pattern:
if (np)
of_mdiobus_register()
else
mdiobus_register()
which the implementation of of_mdiobus_register() now takes care of.
Remove that pattern in drivers that strictly adhere to it.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 8 ++------
drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
drivers/net/ethernet/freescale/fec_main.c | 8 ++------
drivers/net/ethernet/marvell/mvmdio.c | 5 +----
drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
drivers/net/ethernet/ti/davinci_mdio.c | 8 +++-----
drivers/net/phy/mdio-gpio.c | 6 +-----
drivers/net/phy/mdio-mscc-miim.c | 6 +-----
drivers/net/usb/lan78xx.c | 7 ++-----
11 files changed, 20 insertions(+), 61 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index ac621f44237a..02e8982519ce 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -450,12 +450,8 @@ static int bcm_sf2_mdio_register(struct dsa_switch *ds)
priv->slave_mii_bus->parent = ds->dev->parent;
priv->slave_mii_bus->phy_mask = ~priv->indir_phy_mask;
- if (dn)
- err = of_mdiobus_register(priv->slave_mii_bus, dn);
- else
- err = mdiobus_register(priv->slave_mii_bus);
-
- if (err)
+ err = of_mdiobus_register(priv->slave_mii_bus, dn);
+ if (err && dn)
of_node_put(dn);
return err;
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b23c11d9f4b2..2bb3f03ee1cb 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2454,10 +2454,7 @@ static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
return err;
}
- if (np)
- err = of_mdiobus_register(bus, np);
- else
- err = mdiobus_register(bus);
+ err = of_mdiobus_register(bus, np);
if (err) {
dev_err(chip->dev, "Cannot register MDIO bus (%d)\n", err);
mv88e6xxx_g2_irq_mdio_free(chip, bus);
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b4c9268100bb..3e93df5d4e3b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -591,16 +591,10 @@ static int macb_mii_init(struct macb *bp)
dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
np = bp->pdev->dev.of_node;
+ if (pdata)
+ bp->mii_bus->phy_mask = pdata->phy_mask;
- if (np) {
- err = of_mdiobus_register(bp->mii_bus, np);
- } else {
- if (pdata)
- bp->mii_bus->phy_mask = pdata->phy_mask;
-
- err = mdiobus_register(bp->mii_bus);
- }
-
+ err = of_mdiobus_register(bp->mii_bus, np);
if (err)
goto err_out_free_mdiobus;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d4604bc8eb5b..f3e43db0d6cb 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2052,13 +2052,9 @@ static int fec_enet_mii_init(struct platform_device *pdev)
fep->mii_bus->parent = &pdev->dev;
node = of_get_child_by_name(pdev->dev.of_node, "mdio");
- if (node) {
- err = of_mdiobus_register(fep->mii_bus, node);
+ err = of_mdiobus_register(fep->mii_bus, node);
+ if (node)
of_node_put(node);
- } else {
- err = mdiobus_register(fep->mii_bus);
- }
-
if (err)
goto err_out_free_mdiobus;
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 0495487f7b42..c5dac6bd2be4 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -348,10 +348,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
goto out_mdio;
}
- if (pdev->dev.of_node)
- ret = of_mdiobus_register(bus, pdev->dev.of_node);
- else
- ret = mdiobus_register(bus);
+ ret = of_mdiobus_register(bus, pdev->dev.of_node);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
goto out_mdio;
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 5970d9e5ddf1..8dd41e08a6c6 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3025,15 +3025,10 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
pdev->name, pdev->id);
/* register MDIO bus */
- if (dev->of_node) {
- ret = of_mdiobus_register(mdp->mii_bus, dev->of_node);
- } else {
- if (pd->phy_irq > 0)
- mdp->mii_bus->irq[pd->phy] = pd->phy_irq;
-
- ret = mdiobus_register(mdp->mii_bus);
- }
+ if (pd->phy_irq > 0)
+ mdp->mii_bus->irq[pd->phy] = pd->phy_irq;
+ ret = of_mdiobus_register(mdp->mii_bus, dev->of_node);
if (ret)
goto out_free_bus;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index f5f37bfa1d58..5df1a608e566 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -233,10 +233,7 @@ int stmmac_mdio_register(struct net_device *ndev)
new_bus->phy_mask = mdio_bus_data->phy_mask;
new_bus->parent = priv->device;
- if (mdio_node)
- err = of_mdiobus_register(new_bus, mdio_node);
- else
- err = mdiobus_register(new_bus);
+ err = of_mdiobus_register(new_bus, mdio_node);
if (err != 0) {
dev_err(dev, "Cannot register the MDIO bus\n");
goto bus_register_fail;
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 98a1c97fb95e..8ac72831af05 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -429,12 +429,10 @@ static int davinci_mdio_probe(struct platform_device *pdev)
* defined to support backward compatibility with DTs which assume that
* Davinci MDIO will always scan the bus for PHYs detection.
*/
- if (dev->of_node && of_get_child_count(dev->of_node)) {
+ if (dev->of_node && of_get_child_count(dev->of_node))
data->skip_scan = true;
- ret = of_mdiobus_register(data->bus, dev->of_node);
- } else {
- ret = mdiobus_register(data->bus);
- }
+
+ ret = of_mdiobus_register(data->bus, dev->of_node);
if (ret)
goto bail_out;
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index b501221819e1..4e4c8daf44c3 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -179,11 +179,7 @@ static int mdio_gpio_probe(struct platform_device *pdev)
if (!new_bus)
return -ENODEV;
- if (pdev->dev.of_node)
- ret = of_mdiobus_register(new_bus, pdev->dev.of_node);
- else
- ret = mdiobus_register(new_bus);
-
+ ret = of_mdiobus_register(new_bus, pdev->dev.of_node);
if (ret)
mdio_gpio_bus_deinit(&pdev->dev);
diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c
index 8c689ccfdbca..badbc99bedd3 100644
--- a/drivers/net/phy/mdio-mscc-miim.c
+++ b/drivers/net/phy/mdio-mscc-miim.c
@@ -151,11 +151,7 @@ static int mscc_miim_probe(struct platform_device *pdev)
}
}
- if (pdev->dev.of_node)
- ret = of_mdiobus_register(bus, pdev->dev.of_node);
- else
- ret = mdiobus_register(bus);
-
+ ret = of_mdiobus_register(bus, pdev->dev.of_node);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
return ret;
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 91761436709a..8dff87ec6d99 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1843,12 +1843,9 @@ static int lan78xx_mdio_init(struct lan78xx_net *dev)
}
node = of_get_child_by_name(dev->udev->dev.of_node, "mdio");
- if (node) {
- ret = of_mdiobus_register(dev->mdiobus, node);
+ ret = of_mdiobus_register(dev->mdiobus, node);
+ if (node)
of_node_put(node);
- } else {
- ret = mdiobus_register(dev->mdiobus);
- }
if (ret) {
netdev_err(dev->net, "can't register MDIO bus\n");
goto exit1;
--
2.14.1
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add DT binding document for PMIC5 ADC
From: Siddartha Mohanadoss @ 2018-05-15 23:57 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio, devicetree, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Rob Herring, cdevired, rphani, sivaa
In-Reply-To: <20180512111543.4f65dddd@archlinux>
Hi Jonathan,
Thanks for the comments.
On 05/12/2018 03:15 AM, Jonathan Cameron wrote:
> On Tue, 8 May 2018 14:38:21 -0700
> Siddartha Mohanadoss <smohanad@codeaurora.org> wrote:
>
>> PMIC5 ADC has support for clients to measure voltage and current
>> on inputs connected to the PMIC. Clients include reading voltage
>> phone power and on board system thermistors for thermal management.
>> ADC5 on certain PMIC has support to read battery current.
>>
>> This change adds documentation.
>>
>> Signed-off-by: Siddartha Mohanadoss <smohanad@codeaurora.org>
> Hi Siddartha,
>
> Some complexity in here! Anyhow, a few comments inline and we will definitely
> be wanting guidance from the devicetree people for this one.
>
> Jonathan
>
>> ---
>> .../devicetree/bindings/iio/adc/qcom,spmi-adc5.txt | 137 +++++++++++++++++++++
>> 1 file changed, 137 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
>> new file mode 100644
>> index 0000000..c9268ba
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
>> @@ -0,0 +1,137 @@
>> +Qualcomm Technologies Inc. SPMI PMIC5 voltage and current ADC
>> +
>> +SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
>> +voltage. The VADC is a 16-bit sigma-delta ADC.
>> +
>> +ADC node:
>> +
>> +- compatible:
>> + Usage: required
>> + Value type: <string>
>> + Definition: Should contain "qcom,spmi-adc5" for PMIC5 ADC driver.
>> + Should contain "qcom,spmi-adc-rev2" for PMIC refresh ADC driver.
>> +
>> +- reg:
>> + Usage: required
>> + Value type: <prop-encoded-array>
>> + Definition: VADC base address and length in the SPMI PMIC register map.
>> +
>> +- #address-cells:
>> + Usage: required
>> + Value type: <u32>
>> + Definition: Must be one. Child node 'reg' property should define ADC
>> + channel number.
>> +
>> +- #size-cells:
>> + Usage: required
>> + Value type: <u32>
>> + Definition: Must be zero.
>> +
>> +- #io-channel-cells:
>> + Usage: required
>> + Value type: <u32>
>> + Definition: Must be one. For details about IIO bindings see:
>> + Documentation/devicetree/bindings/iio/iio-bindings.txt
>> +
>> +- interrupts:
>> + Usage: optional
>> + Value type: <prop-encoded-array>
>> + Definition: End of conversion interrupt.
>> +
>> +Channel node properties:
>> +
>> +- reg:
>> + Usage: required
>> + Value type: <u32>
>> + Definition: ADC channel number.
>> + See include/dt-bindings/iio/qcom,spmi-vadc.h
>> +
>> +- label:
>> + Usage: required
>> + Value type: <empty>
>> + Definition: ADC datasheet channel name.
>> + For thermistor inputs connected to generic AMUX or GPIO inputs
>> + these can vary across platform for the same pins. Hence select
>> + the datasheet name for this channel.
>> +
>> +- qcom,pre-scaling:
>> + Usage: required
>> + Value type: <u32 array>
>> + Definition: Used for scaling the channel input signal before the signal is
>> + fed to VADC. The configuration for this node is to know the
>> + pre-determined ratio and use it for post scaling. Select one from
>> + the following options.
>> + <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10>
>> + If property is not found default value depending on chip will be used.
>> +
>> +- qcom,decimation:
>> + Usage: optional
>> + Value type: <u32>
>> + Definition: This parameter is used to decrease ADC sampling rate.
>> + Quicker measurements can be made by reducing decimation ratio.
>> + For PMIC5 ADC, combined two step decimation values are 250, 420 and 840.
>> + If property is not found, default value of 840 will be used.
> The odd indenting here needs sorting. Mixture of spaces and tabs at the moment.
Ok, will take a look.
> Hmm. In someways this is a policy decision so should be pushed up to userspace,
> but given the 'right' value will be somewhat dependent on what you are doing
> with the channel and what is wired to it, it could arguably have a 'right' value
> for a given circuit. This is really just the sampling frequency wrapped
> up in decimation of something, I'm guessing some input clock?
Yes. It's number of samples collected over a 4.8MHz clock.
The only reason to update this value would be if client wants to get
the conversion results back sooner. Hence left this as an optional property.
>
> Let's see what the Device-tree people think on this one! Personally I have
> never really minded devicetree providing sensible defaults. We can put
> control on these things later, if there is a usecase for changing them.
>
>> + For PMIC refresh ADC, supported decimation values are 256, 512, 1024.
>> + If property is not found, default value of 1024 will be used.
>> +
>> +- qcom,ratiometric:
>> + Usage: optional
>> + Value type: <empty>
>> + Definition: Channel calibration type. If this property is specified
>> + VADC will use the VDD reference (1.875V) and GND for channel
>> + calibration. If property is not found, channel will be
>> + calibrated with 0V and 1.25V reference channels, also
>> + known as absolute calibration.
>> +
>> +- qcom,hw-settle-time:
>> + Usage: optional
>> + Value type: <u32>
>> + Definition: Time between AMUX getting configured and the ADC starting
>> + conversion.
>> + For PMIC5, delay = 15us for value 0,
>> + 100us * (value) for values 0 < value < 11, and
>> + 2ms * (value - 10) otherwise.
>> + Valid values are: 15, 100, 200, 300, 400, 500, 600, 700, 800,
> This description is very confusing given the different uses of 'value'
> None of the values you have allowed is less than 11 so the first condition
> doesn't apply.
The 'value' is an index programmed in the hardware to achieve the
hardware settling delay specified under valid values. I will update the
documentation here.
>
>> + 900 us and 1, 2, 4, 6, 8, 10 ms
>> + If property is not found, channel will use 15us.
>> + For PMIC rev2, delay = 100us * (value) for values 0 < value < 11, and
>> + 2ms * (value - 10) otherwise.
>> + Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800,
>> + 900 us and 1, 2, 4, 6, 8, 10 ms
>> + If property is not found, channel will use 0 us.
>> +
>> +- qcom,avg-samples:
>> + Usage: optional
>> + Value type: <u32>
>> + Definition: Number of samples to be used for measurement.
>> + Averaging provides the option to obtain a single measurement
>> + from the ADC that is an average of multiple samples. The value
>> + selected is 2^(value).
>> + Valid values are: 1, 2, 4, 8, 16
>> + If property is not found, 1 sample will be used.
> As with decimation, this is arguably not a feature of the hardware, but
> a software decision...
>
>> +
>> +Example:
>> +
>> + /* VADC node */
>> + pmic_vadc: vadc@3100 {
>> + compatible = "qcom,spmi-adc5";
>> + reg = <0x3100 0x100>;
>> + interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + #io-channel-cells = <1>;
>> + io-channel-ranges;
>> +
>> + /* Channel node */
>> + vph_pwr {
>> + reg = <ADC_VPH_PWR>;
>> + label = "vph_pwr";
>> + qcom,pre-scaling = <1 3>;
>> + };
>> + };
>> +
>> + /* IIO client node */
>> + usb {
>> + io-channels = <&pmic_vadc ADC_VPH_PWR>;
>> + io-channel-names = "vadc";
>> + };
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] drivers: net: Remove device_node checks with of_mdiobus_register()
From: Grygorii Strashko @ 2018-05-16 0:01 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, Nicolas Ferre,
Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan, Russell King, Geert Uytterhoeven,
Thomas
In-Reply-To: <20180515235619.27773-3-f.fainelli@gmail.com>
On 05/15/2018 06:56 PM, Florian Fainelli wrote:
> A number of drivers have the following pattern:
>
> if (np)
> of_mdiobus_register()
> else
> mdiobus_register()
>
> which the implementation of of_mdiobus_register() now takes care of.
> Remove that pattern in drivers that strictly adhere to it.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/dsa/bcm_sf2.c | 8 ++------
> drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
> drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
> drivers/net/ethernet/freescale/fec_main.c | 8 ++------
> drivers/net/ethernet/marvell/mvmdio.c | 5 +----
> drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
> drivers/net/ethernet/ti/davinci_mdio.c | 8 +++-----
for drivers/net/ethernet/ti/davinci_mdio.c:
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
> drivers/net/phy/mdio-gpio.c | 6 +-----
> drivers/net/phy/mdio-mscc-miim.c | 6 +-----
> drivers/net/usb/lan78xx.c | 7 ++-----
> 11 files changed, 20 insertions(+), 61 deletions(-)
--
regards,
-grygorii
^ 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