Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 08/10] ARM: dts: sun8i-a33-sinlinx-sina33: use AXP223 DTSI
From: Chen-Yu Tsai @ 2016-11-25 10:05 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Sebastian Reichel, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Russell King, Maxime Ripard, Lee Jones, open list:THERMAL,
	devicetree, linux-kernel, linux-arm-kernel, Thomas Petazzoni
In-Reply-To: <20161125090921.23138-9-quentin.schulz@free-electrons.com>

On Fri, Nov 25, 2016 at 5:09 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> Previously, the Sinlinx SinA33 used everything declared in AXP221 DTSI
> while it has an AXP223 PMIC.
>
> This corrects that so the Sinlinx SinA33 can get some features the
> AXP223 has (at the moment, ability to have 100mA as maximal current on
> VBUS power supply).
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

^ permalink raw reply

* Re: [PATCH 07/10] ARM: dts: sun8i-a33-olinuxino: use AXP223 DTSI
From: Chen-Yu Tsai @ 2016-11-25 10:05 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Mark Rutland, devicetree, open list:THERMAL, linux-kernel,
	Sebastian Reichel, Russell King, Chen-Yu Tsai, Rob Herring,
	Maxime Ripard, Lee Jones, Thomas Petazzoni, linux-arm-kernel
In-Reply-To: <20161125090921.23138-8-quentin.schulz@free-electrons.com>

On Fri, Nov 25, 2016 at 5:09 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> Previously, the Olimex A33-OlinuXino used everything declared in AXP221
> DTSI while it has an AXP223 PMIC.
>
> This corrects that so the Olimex A33-OlinuXino can get some features the
> AXP223 has (at the moment, ability to have 100mA as maximal current on
> VBUS power supply).
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

^ permalink raw reply

* Re: [PATCH 06/10] ARM: dtsi: add DTSI for AXP223
From: Chen-Yu Tsai @ 2016-11-25 10:04 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Mark Rutland, devicetree, open list:THERMAL, linux-kernel,
	Sebastian Reichel, Russell King, Chen-Yu Tsai, Rob Herring,
	Maxime Ripard, Lee Jones, Thomas Petazzoni, linux-arm-kernel
In-Reply-To: <20161125090921.23138-7-quentin.schulz@free-electrons.com>

On Fri, Nov 25, 2016 at 5:09 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> The AXP223 shares most of its logic with the AXP221 but it has some
> differences for the VBUS driver.

You could also mention this in the dtsi file itself.

Otherwise,

Acked-by: Chen-Yu Tsai <wens@csie.org>

>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
>  arch/arm/boot/dts/axp223.dtsi | 55 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 arch/arm/boot/dts/axp223.dtsi
>
> diff --git a/arch/arm/boot/dts/axp223.dtsi b/arch/arm/boot/dts/axp223.dtsi
> new file mode 100644
> index 0000000..f8ce55c
> --- /dev/null
> +++ b/arch/arm/boot/dts/axp223.dtsi
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright 2016 Free Electrons
> + *
> + * Quentin Schulz <quentin.schulz@free-electrons.com>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/*
> + * AXP223 Integrated Power Management Chip
> + * http://www.x-powers.com/product/AXP22X.php
> + * http://dl.linux-sunxi.org/AXP/AXP223-en.pdf
> + */
> +
> +#include "axp22x.dtsi"
> +
> +&usb_power_supply {
> +       compatible = "x-powers,axp223-usb-power-supply";
> +};
> --
> 2.9.3
>

^ permalink raw reply

* Re: [PATCH 05/10] mfd: axp20x: add separate MFD cell for AXP223
From: Chen-Yu Tsai @ 2016-11-25 10:03 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Mark Rutland, devicetree, open list:THERMAL, linux-kernel,
	Sebastian Reichel, Russell King, Chen-Yu Tsai, Rob Herring,
	Maxime Ripard, Lee Jones, Thomas Petazzoni, linux-arm-kernel
In-Reply-To: <20161125090921.23138-6-quentin.schulz@free-electrons.com>

On Fri, Nov 25, 2016 at 5:09 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> The AXP223 shares most of its logic with the AXP221 but has some
> differences for the VBUS power supply driver. Thus, to probe the driver
> with the correct compatible, the AXP221 and the AXP223 now have separate
> MFD cells.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
>  drivers/mfd/axp20x.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index ba130be..989d568 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -602,6 +602,21 @@ static struct mfd_cell axp22x_cells[] = {
>         },
>  };
>
> +static struct mfd_cell axp223_cells[] = {
> +       {
> +               .name                   = "axp20x-pek",
> +               .num_resources          = ARRAY_SIZE(axp22x_pek_resources),
> +               .resources              = axp22x_pek_resources,
> +       }, {
> +               .name                   = "axp20x-regulator",
> +       }, {
> +               .name           = "axp20x-usb-power-supply",
> +               .of_compatible  = "x-powers,axp223-usb-power-supply",
> +               .num_resources  = ARRAY_SIZE(axp22x_usb_power_supply_resources),
> +               .resources      = axp22x_usb_power_supply_resources,

Nit: Please align the statements. And you might want to rename the original
axp22x_cells to axp221_cells to avoid confusion. Otherwise,

Acked-by: Chen-Yu Tsai <wens@csie.org>

> +       },
> +};
> +
>  static struct mfd_cell axp152_cells[] = {
>         {
>                 .name                   = "axp20x-pek",
> @@ -789,12 +804,17 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
>                 axp20x->regmap_irq_chip = &axp20x_regmap_irq_chip;
>                 break;
>         case AXP221_ID:
> -       case AXP223_ID:
>                 axp20x->nr_cells = ARRAY_SIZE(axp22x_cells);
>                 axp20x->cells = axp22x_cells;
>                 axp20x->regmap_cfg = &axp22x_regmap_config;
>                 axp20x->regmap_irq_chip = &axp22x_regmap_irq_chip;
>                 break;
> +       case AXP223_ID:
> +               axp20x->nr_cells = ARRAY_SIZE(axp223_cells);
> +               axp20x->cells = axp223_cells;
> +               axp20x->regmap_cfg = &axp22x_regmap_config;
> +               axp20x->regmap_irq_chip = &axp22x_regmap_irq_chip;
> +               break;
>         case AXP288_ID:
>                 axp20x->cells = axp288_cells;
>                 axp20x->nr_cells = ARRAY_SIZE(axp288_cells);
> --
> 2.9.3
>

^ permalink raw reply

* Re: [PATCH v2 0/3] increase TSCADC clock to 24MHz and fix ti,charge-delay to represent in nS
From: Lee Jones @ 2016-11-25  9:59 UTC (permalink / raw)
  To: Mugunthan V N
  Cc: linux-input, Dmitry Torokhov, Jonathan Cameron, Rob Herring,
	Mark Rutland, Sekhar Nori, Vignesh R, devicetree, linux-omap,
	linux-kernel
In-Reply-To: <71482721-fd60-65b2-d601-f1698ce5fa0a@ti.com>

On Fri, 25 Nov 2016, Mugunthan V N wrote:

> Hi Dmitry Torokhov,
> 
> On Thursday 10 November 2016 10:05 PM, Mugunthan V N wrote:
> > This patch series enables ADC to be clocked at 24MHz as the
> > TI AM335x ADC driver has already adopted to use DMA to transfer
> > ADC samples. Now ADC can generated upto 800K Samples per second
> > with the patch [1] on AM335x BBB and AM437x GP EVM.
> > 
> > when ADC ref clock is set at 24MHz, I am seeing some issue with
> > touch screen pointer as the pointer jumps to random locations
> > with free draw application. The issue is due to increase in ADC
> > clock and charge delay for the touchscreen ADC line duration
> > reduced.
> > 
> > So the notation of ti,charge-delay in terms of ADC clock is
> > wrong, it has to be represented in time and driver has to convert
> > the charge delay time to ADC clocks based on what ADC clock
> > frequency is set.
> > 
> > Measured the performance with the iio_generic_buffer with the
> > patch [2] applied
> > 
> > Verified the touch screen on AM335x GP EVM and AM335x BBB LCD7
> > cape with [3] dts for display and touch screen to work.
> > 
> 
> Since there are acks from DT and MFD maintainers, can you pull the patch
> series if you do not have any more comments.

Cant do anything without *all* Acks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH V4 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Thierry Reding @ 2016-11-25  9:57 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	jonathanh-DDmLM1+adcrQT0dZR+AlfA, joe-6d6DIl74uiNBDgjK7y7TUQ,
	yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479976734-30498-3-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 24860 bytes --]

On Thu, Nov 24, 2016 at 02:08:54PM +0530, Laxman Dewangan wrote:
> NVIDIA Tegra124 and later SoCs support the multi-voltage level and
> low power state of some of its IO pads. The IO pads can work in
> the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
> sources. When IO interfaces are not used then IO pads can be
> configure in low power state to reduce the power consumption from
> that IO pads.
> 
> On Tegra124, the voltage level of IO power rail source is auto
> detected by hardware(SoC) and hence it is only require to configure
> in low power mode if IO pads are not used.
> 
> On T210 onwards, the auto-detection of voltage level from IO power
> rail is removed from SoC and hence SW need to configure the PMC
> register explicitly to set proper voltage in IO pads based on
> IO rail power source voltage.
> 
> This driver adds the IO pad driver to configure the power state and
> IO pad voltage based on the usage and power tree via pincontrol
> framework. The configuration can be static and dynamic.
> 
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> ---
> Changes from V1:
> - Dropped the custom properties to set pad voltage and use regulator.
> - Added support for regulator to get vottage in boot and configure IO
>   pad voltage.
> - Add support for callback to handle regulator notification and configure
>   IO pad voltage based on voltage change.
> 
> Changes from V2:
>  Mostly nit changes per Jon's feedback i.e. use macros for voltage, added
>  comment on macros, reduce the structure and variable name size, optimise
>  number of variables, and allocate memory for regulator info when it needed.
> 
> Changes from V3:
>  Use the devm_regulator_get() instead of devm_regulator_get_optional().
> 
>  drivers/pinctrl/tegra/Kconfig                |  12 +
>  drivers/pinctrl/tegra/Makefile               |   1 +
>  drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c | 530 +++++++++++++++++++++++++++
>  3 files changed, 543 insertions(+)
>  create mode 100644 drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
> 
> diff --git a/drivers/pinctrl/tegra/Kconfig b/drivers/pinctrl/tegra/Kconfig
> index 24e20cc..6004e5c 100644
> --- a/drivers/pinctrl/tegra/Kconfig
> +++ b/drivers/pinctrl/tegra/Kconfig
> @@ -23,6 +23,18 @@ config PINCTRL_TEGRA210
>  	bool
>  	select PINCTRL_TEGRA
>  
> +config PINCTRL_TEGRA_IO_PAD
> +	bool "Tegra IO pad Control Driver"
> +	depends on ARCH_TEGRA && REGULATOR
> +	select PINCONF
> +	select PINMUX
> +	help
> +	  NVIDIA Tegra124/210 SoC has IO pads which supports multi-voltage
> +	  level of interfacing and deep power down mode of IO pads. The
> +	  voltage of IO pads are SW configurable based on IO rail of that
> +	  pads on T210. This driver provides the interface to change IO pad
> +	  voltage and power state via pincontrol interface.

This has a lot of chip-specific text. Will all of that have to be
updated if support for new chips is added?

> +
>  config PINCTRL_TEGRA_XUSB
>  	def_bool y if ARCH_TEGRA
>  	select GENERIC_PHY
> diff --git a/drivers/pinctrl/tegra/Makefile b/drivers/pinctrl/tegra/Makefile
> index d9ea2be..3ebaaa2 100644
> --- a/drivers/pinctrl/tegra/Makefile
> +++ b/drivers/pinctrl/tegra/Makefile
> @@ -4,4 +4,5 @@ obj-$(CONFIG_PINCTRL_TEGRA30)		+= pinctrl-tegra30.o
>  obj-$(CONFIG_PINCTRL_TEGRA114)		+= pinctrl-tegra114.o
>  obj-$(CONFIG_PINCTRL_TEGRA124)		+= pinctrl-tegra124.o
>  obj-$(CONFIG_PINCTRL_TEGRA210)		+= pinctrl-tegra210.o
> +obj-$(CONFIG_PINCTRL_TEGRA_IO_PAD)	+= pinctrl-tegra-io-pad.o
>  obj-$(CONFIG_PINCTRL_TEGRA_XUSB)	+= pinctrl-tegra-xusb.o
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
> new file mode 100644
> index 0000000..aab02d0
> --- /dev/null
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
> @@ -0,0 +1,530 @@
> +/*
> + * pinctrl-tegra-io-pad: IO PAD driver for configuration of IO rail and deep
> + *			 Power Down mode via pinctrl framework.
> + *
> + * Copyright (C) 2016 NVIDIA CORPORATION. All rights reserved.
> + *
> + * Author: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinconf.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <soc/tegra/pmc.h>

Have you considered moving this code into the PMC driver? It seems a
little over the top to go through all of the platform device creation
and driver registration dance only to call into a public API later on.

> +#include "../core.h"
> +#include "../pinconf.h"
> +#include "../pinctrl-utils.h"
> +
> +#define TEGRA_IO_RAIL_1800000UV 1800000
> +#define TEGRA_IO_RAIL_3300000UV 3300000

Is there really a point in having these defines? They are really long
and effectively don't carry more information than just the plain
numbers.

> +
> +/* Covert IO voltage to IO pad voltage enum */

Convert

> +#define tegra_io_uv_to_io_pads_uv(io_uv)				\
> +		(((io_uv) == TEGRA_IO_RAIL_1800000UV) ?			\
> +		  TEGRA_IO_PAD_1800000UV : TEGRA_IO_PAD_3300000UV)
> +
> +#define tegra_io_voltage_is_valid(io_uv)			\
> +	({ typeof(io_uv) io_uv_ = (io_uv);			\
> +	    ((io_uv_ == TEGRA_IO_RAIL_1800000UV) ||		\
> +	     (io_uv_ == TEGRA_IO_RAIL_3300000UV)); })
>
Maybe make both of these static inline functions to improve readability?
I find the above very hard to read.

> +struct tegra_io_pads_cfg {
> +	const char *name;
> +	const unsigned int pins[1];
> +	const char *vsupply;
> +	enum tegra_io_pad id;
> +	bool supports_low_power;
> +};

Pretty much everything in this driver operates on single pads, so it's a
little confusing to use the "pads" in the names. I think it would be
more appropriate to name this structure tegra_io_pad_cfg because it is
configuration data associated with a single pad.

> +
> +struct tegra_io_pads_soc_data {

I think the _data suffix is redundant and can be dropped.

The use of "pads" in this structure name is fine because it really does
contain data for multiple pads.

> +	const struct tegra_io_pads_cfg *cfg;
> +	int num_cfg;

This can be unsigned int. Also I think it's more common to use the
plural in these (cfgs, num_cfgs).

> +	const struct pinctrl_pin_desc *desc;
> +	int num_desc;

Same here.

> +};
> +
> +struct tegra_io_pads_info {
> +	struct device *dev;
> +	struct pinctrl_dev *pctl;
> +	const struct tegra_io_pads_soc_data *soc_data;

If you drop the _data suffix from the type I think you can also drop it
from the field name.

"pads" is fine here as well because, again, this deals with multiple
pads.

> +};
> +
> +struct tegra_io_pads_regulator_info {
> +	struct tegra_io_pads_info *tiopi;
> +	const struct tegra_io_pads_cfg *cfg;
> +	struct regulator *regulator;
> +	struct notifier_block regulator_nb;

The regulator_ prefix is redundent here. It's contained within a
structure named tegra_io_pads_regulator_info, so it's fairly obvious
that this is related to regulators.

This deals only with a single pad, so tegra_io_pad_regulator_info would
be less confusing, I think. Perhaps even drop the _info suffix while at
it because it doesn't add anything useful.

> +};
> +
> +static int tegra_io_pads_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
> +{
> +	struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
> +
> +	return tiopi->soc_data->num_cfg;
> +}
> +
> +static const char *tegra_io_pads_pinctrl_get_group_name(
> +		struct pinctrl_dev *pctldev, unsigned int group)
> +{
> +	struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
> +
> +	return tiopi->soc_data->cfg[group].name;
> +}
> +
> +static int tegra_io_pads_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
> +						unsigned int group,
> +						const unsigned int **pins,
> +						unsigned int *num_pins)
> +{
> +	struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
> +
> +	*pins = tiopi->soc_data->cfg[group].pins;
> +	*num_pins = 1;
> +
> +	return 0;
> +}
> +
> +static const struct pinctrl_ops tegra_io_pads_pinctrl_ops = {
> +	.get_groups_count	= tegra_io_pads_pinctrl_get_groups_count,
> +	.get_group_name		= tegra_io_pads_pinctrl_get_group_name,
> +	.get_group_pins		= tegra_io_pads_pinctrl_get_group_pins,
> +	.dt_node_to_map		= pinconf_generic_dt_node_to_map_pin,
> +	.dt_free_map		= pinctrl_utils_free_map,
> +};

Nit: I don't think this padding using tabs increases readability.

> +
> +static int tegra_io_pads_pinconf_get(struct pinctrl_dev *pctldev,
> +				     unsigned int pin, unsigned long *config)
> +{
> +	struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
> +	int param = pinconf_to_config_param(*config);
> +	const struct tegra_io_pads_cfg *cfg = &tiopi->soc_data->cfg[pin];
> +	int arg = 0;

Why not make that a u16...

> +	int ret;
> +
> +	switch (param) {
> +	case PIN_CONFIG_LOW_POWER_MODE:
> +		if (!cfg->supports_low_power) {
> +			dev_err(tiopi->dev,
> +				"IO pad %s does not support low power\n",
> +				cfg->name);
> +			return -EINVAL;
> +		}
> +
> +		ret = tegra_io_pad_power_get_status(cfg->id);
> +		if (ret < 0)
> +			return ret;
> +		arg = !ret;
> +		break;
> +
> +	default:
> +		dev_err(tiopi->dev, "The parameter %d not supported\n", param);
> +		return -EINVAL;
> +	}
> +
> +	*config = pinconf_to_config_packed(param, (u16)arg);

... and avoid the cast here?

> +
> +	return 0;
> +}
> +
> +static int tegra_io_pads_pinconf_set(struct pinctrl_dev *pctldev,
> +				     unsigned int pin, unsigned long *configs,
> +				    unsigned int num_configs)

This last line is not quite properly aligned.

> +{
> +	struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
> +	const struct tegra_io_pads_cfg *cfg = &tiopi->soc_data->cfg[pin];
> +	int i;

This should be unsigned.

> +
> +	for (i = 0; i < num_configs; i++) {
> +		int ret;
> +		int param = pinconf_to_config_param(configs[i]);

The function returns an enum pin_config_param, why not use that as the
type?

> +		u16 param_val = pinconf_to_config_argument(configs[i]);
> +
> +		switch (param) {
> +		case PIN_CONFIG_LOW_POWER_MODE:
> +			if (!cfg->supports_low_power) {
> +				dev_err(tiopi->dev,
> +					"IO pad %s does not support low power\n",
> +					cfg->name);
> +				return -EINVAL;
> +			}
> +			if (param_val)
> +				ret = tegra_io_pad_power_disable(cfg->id);
> +			else
> +				ret = tegra_io_pad_power_enable(cfg->id);
> +			if (ret < 0) {
> +				dev_err(tiopi->dev,
> +					"Failed to set DPD %d of io-pad %s: %d\n",
> +					param_val, cfg->name, ret);
> +				return ret;
> +			}
> +			break;
> +
> +		default:
> +			dev_err(tiopi->dev, "The parameter %d not supported\n",
> +				param);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pinconf_ops tegra_io_pads_pinconf_ops = {
> +	.pin_config_get = tegra_io_pads_pinconf_get,
> +	.pin_config_set = tegra_io_pads_pinconf_set,
> +};
> +
> +static struct pinctrl_desc tegra_io_pads_pinctrl_desc = {
> +	.name = "pinctrl-tegra-io-pads",
> +	.pctlops = &tegra_io_pads_pinctrl_ops,
> +	.confops = &tegra_io_pads_pinconf_ops,
> +};
> +
> +static int tegra_io_pads_rail_change_notify_cb(struct notifier_block *nb,
> +					       unsigned long event, void *data)
> +{
> +	struct tegra_io_pads_regulator_info *rinfo;
> +	struct pre_voltage_change_data *vdata;
> +	unsigned long int io_volt_uv;

You can drop the int, it's implied by long.

> +	enum tegra_io_pad_voltage pad_volt;
> +	int ret;
> +
> +	rinfo = container_of(nb, struct tegra_io_pads_regulator_info,
> +			     regulator_nb);
> +
> +	switch (event) {
> +	case REGULATOR_EVENT_PRE_VOLTAGE_CHANGE:
> +		vdata = data;
> +
> +		if (!tegra_io_voltage_is_valid(vdata->old_uV) ||
> +		    !tegra_io_voltage_is_valid(vdata->min_uV)) {
> +			dev_err(rinfo->tiopi->dev,
> +				"IO rail %s voltage is not 1.8/3.3V: %lu:%lu\n",
> +				rinfo->cfg->name, vdata->old_uV, vdata->min_uV);
> +			return -EINVAL;
> +		}
> +
> +		/**
> +		 * Change IO pad voltage before changing IO voltage when it
> +		 * changes from 1.8V to 3.3V
> +		 */
> +		if (vdata->min_uV == TEGRA_IO_RAIL_1800000UV)
> +			break;
> +
> +		ret = tegra_io_pad_set_voltage(rinfo->cfg->id,
> +					       TEGRA_IO_PAD_3300000UV);
> +		if (ret < 0) {
> +			dev_err(rinfo->tiopi->dev,
> +				"Failed to set voltage %lu of pad %s: %d\n",
> +				vdata->min_uV, rinfo->cfg->name, ret);
> +			return ret;
> +		}
> +		break;
> +
> +	case REGULATOR_EVENT_VOLTAGE_CHANGE:
> +		io_volt_uv = (unsigned long)data;
> +		ret = tegra_io_pad_get_voltage(rinfo->cfg->id);
> +		if (ret < 0) {
> +			dev_err(rinfo->tiopi->dev,
> +				"Failed to get IO pad voltage: %d\n", ret);
> +			return ret;
> +		}

Might be worth reassigning ret to a variable of type enum
tegra_io_pad_voltage because...

> +
> +		if (!tegra_io_voltage_is_valid(io_volt_uv)) {
> +			dev_err(rinfo->tiopi->dev,
> +				"IO rail %s voltage is not 1.8/3.3V: %lu\n",
> +				rinfo->cfg->name, io_volt_uv);
> +			return -EINVAL;
> +		}
> +
> +		/*
> +		 * If IO pad configuration matching with IO rail voltage then
> +		 * do nothing.
> +		 */
> +		if (((io_volt_uv == TEGRA_IO_RAIL_1800000UV) &&
> +		     (ret == TEGRA_IO_PAD_1800000UV)) ||
> +		     ((io_volt_uv == TEGRA_IO_RAIL_3300000UV) &&
> +		      (ret == TEGRA_IO_PAD_3300000UV)))
> +			break;

... if somebody ever inserted code between the above and this, they
might be overwriting ret.

> +
> +		ret = tegra_io_pad_set_voltage(rinfo->cfg->id,
> +					       TEGRA_IO_PAD_1800000UV);
> +		if (ret < 0) {
> +			dev_err(rinfo->tiopi->dev,
> +				"Failed to set voltage %lu of pad %s: %d\n",
> +				vdata->min_uV, rinfo->cfg->name, ret);

You might want to add the units of the voltage to avoid confusion. Same
in a couple more messages above and below.

> +			return ret;
> +		}
> +		break;
> +
> +	case REGULATOR_EVENT_ABORT_VOLTAGE_CHANGE:
> +		io_volt_uv = (unsigned long)data;
> +
> +		if (!tegra_io_voltage_is_valid(io_volt_uv)) {
> +			dev_err(rinfo->tiopi->dev,
> +				"IO rail %s voltage is not 1.8/3.3V: %lu\n",
> +				rinfo->cfg->name, io_volt_uv);
> +			return -EINVAL;
> +		}
> +
> +		pad_volt = tegra_io_uv_to_io_pads_uv(io_volt_uv);
> +		ret = tegra_io_pad_set_voltage(rinfo->cfg->id, pad_volt);
> +		if (ret < 0) {
> +			dev_err(rinfo->tiopi->dev,
> +				"Failed to set voltage %lu of pad %s: %d\n",
> +				io_volt_uv, rinfo->cfg->name, ret);
> +			return ret;
> +		}
> +		break;
> +
> +	default:
> +		break;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static int tegra_io_pads_pinctrl_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const struct platform_device_id *id = platform_get_device_id(pdev);
> +	const struct tegra_io_pads_soc_data *soc_data =
> +			(const struct tegra_io_pads_soc_data *)id->driver_data;
> +	struct tegra_io_pads_info *tiopi;
> +	int ret, i;
> +
> +	if (!pdev->dev.parent->of_node) {
> +		dev_err(dev, "PMC should be register from DT\n");
> +		return -ENODEV;
> +	}
> +
> +	tiopi = devm_kzalloc(dev, sizeof(*tiopi), GFP_KERNEL);
> +	if (!tiopi)
> +		return -ENOMEM;
> +
> +	tiopi->dev = &pdev->dev;
> +	pdev->dev.of_node = pdev->dev.parent->of_node;
> +	tiopi->soc_data = soc_data;
> +
> +	for (i = 0; i < soc_data->num_cfg; ++i) {
> +		struct tegra_io_pads_regulator_info *rinfo;
> +		enum tegra_io_pad_voltage pad_volt;
> +		int io_volt_uv;
> +
> +		if (!soc_data->cfg[i].vsupply)
> +			continue;
> +
> +		rinfo = devm_kzalloc(dev, sizeof(*rinfo), GFP_KERNEL);
> +		if (!rinfo)
> +			return -ENOMEM;
> +
> +		rinfo->tiopi = tiopi;
> +		rinfo->cfg = &soc_data->cfg[i];
> +
> +		rinfo->regulator = devm_regulator_get(dev,
> +						      soc_data->cfg[i].vsupply);
> +		if (IS_ERR(rinfo->regulator)) {
> +			ret = PTR_ERR(rinfo->regulator);
> +			if (ret == -EPROBE_DEFER)
> +				return ret;
> +			continue;
> +		}
> +
> +		io_volt_uv = regulator_get_voltage(rinfo->regulator);
> +		if (io_volt_uv < 0) {
> +			dev_warn(dev, "Failed to get voltage for rail %s: %d\n",
> +				 soc_data->cfg[i].vsupply, io_volt_uv);
> +			continue;
> +		}
> +
> +		if (!tegra_io_voltage_is_valid(io_volt_uv)) {
> +			dev_warn(dev, "IO rail %s voltage is not 1.8/3.3V: %d\n",
> +				 soc_data->cfg[i].vsupply, io_volt_uv);
> +			continue;
> +		}
> +
> +		pad_volt = tegra_io_uv_to_io_pads_uv(io_volt_uv);
> +		ret = tegra_io_pad_set_voltage(soc_data->cfg[i].id, pad_volt);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to set voltage %d of pad %s: %d\n",
> +				io_volt_uv, soc_data->cfg[i].name, ret);
> +			return ret;
> +		}
> +
> +		rinfo->regulator_nb.notifier_call =
> +					tegra_io_pads_rail_change_notify_cb;
> +		ret = devm_regulator_register_notifier(rinfo->regulator,
> +						       &rinfo->regulator_nb);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to register regulator %s notifier: %d\n",
> +				soc_data->cfg[i].name, ret);
> +			return ret;
> +		}
> +	}
> +
> +	tegra_io_pads_pinctrl_desc.pins = tiopi->soc_data->desc;
> +	tegra_io_pads_pinctrl_desc.npins = tiopi->soc_data->num_desc;

This modifies global data. Can we avoid that? I think the easiest would
be to make tegra_io_pads_pinctrl_desc a field of the tegra_io_pads_info
struct.

> +	platform_set_drvdata(pdev, tiopi);
> +
> +	tiopi->pctl = devm_pinctrl_register(dev, &tegra_io_pads_pinctrl_desc,
> +					    tiopi);
> +	if (IS_ERR(tiopi->pctl)) {
> +		ret = PTR_ERR(tiopi->pctl);
> +		dev_err(dev, "Failed to register io-pad pinctrl driver: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +#define TEGRA124_PAD_INFO_TABLE(_entry_)			\
> +	_entry_(0, "audio", AUDIO, true, NULL),			\
> +	_entry_(1, "bb", BB, true, NULL),			\
> +	_entry_(2, "cam", CAM, true, NULL),			\
> +	_entry_(3, "comp", COMP, true, NULL),			\
> +	_entry_(4, "csia", CSIA, true, NULL),			\
> +	_entry_(5, "csib", CSIB, true, NULL),			\
> +	_entry_(6, "csie", CSIE, true, NULL),			\
> +	_entry_(7, "dsi", DSI, true, NULL),			\
> +	_entry_(8, "dsib", DSIB, true, NULL),			\
> +	_entry_(9, "dsic", DSIC, true, NULL),			\
> +	_entry_(10, "dsid", DSID, true, NULL),			\
> +	_entry_(11, "hdmi", HDMI, true, NULL),			\
> +	_entry_(12, "hsic", HSIC, true, NULL),			\
> +	_entry_(13, "hv", HV, true, NULL),			\
> +	_entry_(14, "lvds", LVDS, true, NULL),			\
> +	_entry_(15, "mipi-bias", MIPI_BIAS, true, NULL),	\
> +	_entry_(16, "nand", NAND, true, NULL),			\
> +	_entry_(17, "pex-bias", PEX_BIAS, true, NULL),		\
> +	_entry_(18, "pex-clk1", PEX_CLK1, true, NULL),		\
> +	_entry_(19, "pex-clk2", PEX_CLK2, true, NULL),		\
> +	_entry_(20, "pex-ctrl", PEX_CNTRL, true, NULL),		\
> +	_entry_(21, "sdmmc1", SDMMC1, true, NULL),		\
> +	_entry_(22, "sdmmc3", SDMMC3, true, NULL),		\
> +	_entry_(23, "sdmmc4", SDMMC4, true, NULL),		\
> +	_entry_(24, "sys-ddc", SYS_DDC, true, NULL),		\
> +	_entry_(25, "uart", UART, true, NULL),			\
> +	_entry_(26, "usb0", USB0, true, NULL),			\
> +	_entry_(27, "usb1", USB1, true, NULL),			\
> +	_entry_(28, "usb2", USB2, true, NULL),			\
> +	_entry_(29, "usb-bias", USB_BIAS, true, NULL)
> +
> +#define TEGRA210_PAD_INFO_TABLE(_entry_)			\
> +	_entry_(0, "audio", AUDIO, true, "vddio-audio"),	\
> +	_entry_(1, "audio-hv", AUDIO_HV, true, "vddio-audio-hv"), \
> +	_entry_(2, "cam", CAM, true, "vddio-cam"),		\
> +	_entry_(3, "csia", CSIA, true, NULL),			\
> +	_entry_(4, "csib", CSIB, true, NULL),			\
> +	_entry_(5, "csic", CSIC, true, NULL),			\
> +	_entry_(6, "csid", CSID, true, NULL),			\
> +	_entry_(7, "csie", CSIE, true, NULL),			\
> +	_entry_(8, "csif", CSIF, true, NULL),			\
> +	_entry_(9, "dbg", DBG, true, "vddio-dbg"),		\
> +	_entry_(10, "debug-nonao", DEBUG_NONAO, true, NULL),	\
> +	_entry_(11, "dmic", DMIC, true, "vddio-dmic"),		\
> +	_entry_(12, "dp", DP, true, NULL),			\
> +	_entry_(13, "dsi", DSI, true, NULL),			\
> +	_entry_(14, "dsib", DSIB, true, NULL),			\
> +	_entry_(15, "dsic", DSIC, true, NULL),			\
> +	_entry_(16, "dsid", DSID, true, NULL),			\
> +	_entry_(17, "emmc", SDMMC4, true, NULL),		\
> +	_entry_(18, "emmc2", EMMC2, true, NULL),		\
> +	_entry_(19, "gpio", GPIO, true, "vddio-gpio"),		\
> +	_entry_(20, "hdmi", HDMI, true, NULL),			\
> +	_entry_(21, "hsic", HSIC, true, NULL),			\
> +	_entry_(22, "lvds", LVDS, true, NULL),			\
> +	_entry_(23, "mipi-bias", MIPI_BIAS, true, NULL),	\
> +	_entry_(24, "pex-bias", PEX_BIAS, true, NULL),		\
> +	_entry_(25, "pex-clk1", PEX_CLK1, true, NULL),		\
> +	_entry_(26, "pex-clk2", PEX_CLK2, true, NULL),		\
> +	_entry_(27, "pex-ctrl", PEX_CNTRL, false, "vddio-pex-ctrl"), \
> +	_entry_(28, "sdmmc1", SDMMC1, true, "vddio-sdmmc1"),	\
> +	_entry_(29, "sdmmc3", SDMMC3, true, "vddio-sdmmc3"),	\
> +	_entry_(30, "spi", SPI, true, "vddio-spi"),		\
> +	_entry_(31, "spi-hv", SPI_HV, true, "vddio-spi-hv"),	\
> +	_entry_(32, "uart", UART, true, "vddio-uart"),		\
> +	_entry_(33, "usb0", USB0, true, NULL),			\
> +	_entry_(34, "usb1", USB1, true, NULL),			\
> +	_entry_(35, "usb2", USB2, true, NULL),			\
> +	_entry_(36, "usb3", USB3, true, NULL),			\
> +	_entry_(37, "usb-bias", USB_BIAS, true, NULL)
> +
> +#define TEGRA_IO_PAD_INFO(_pin, _name, _id, _lpstate, _vsupply)	\
> +	{							\
> +		.name = _name,					\
> +		.pins = {(_pin)},				\
> +		.id = TEGRA_IO_PAD_##_id,			\
> +		.vsupply = (_vsupply),				\
> +		.supports_low_power = (_lpstate),		\
> +	}
> +
> +static const struct tegra_io_pads_cfg tegra124_io_pads_cfg_info[] = {
> +	TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
> +};
> +
> +static const struct tegra_io_pads_cfg tegra210_io_pads_cfg_info[] = {
> +	TEGRA210_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
> +};

That's a weird way of writing these tables. Why not do something simpler
and much more common such as:

	#define TEGRA_IO_PAD_INFO(...) ...

	static const struct tegra_io_pads_cfg tegra124_io_pads_cfgs[] = {
		TEGRA_IO_PAD_INFO(...),
		...
	};

	static const struct tegra_io_pads_cfg tegra210_io_pad_cfgs[] = {
		TEGRA_IO_PAD_INFO(...),
		...
	};

> +
> +#define TEGRA_IO_PAD_DESC(_pin, _name, _id, _lpstate, _vsupply)	\
> +	PINCTRL_PIN(_pin, _name)
> +
> +static const struct pinctrl_pin_desc tegra124_io_pads_pinctrl_desc[] = {
> +	TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_DESC),
> +};
> +
> +static const struct pinctrl_pin_desc tegra210_io_pads_pinctrl_desc[] = {
> +	TEGRA210_PAD_INFO_TABLE(TEGRA_IO_PAD_DESC),
> +};
> +
> +static const struct tegra_io_pads_soc_data tegra124_io_pad_soc_data = {
> +	.desc		= tegra124_io_pads_pinctrl_desc,
> +	.num_desc	= ARRAY_SIZE(tegra124_io_pads_pinctrl_desc),
> +	.cfg		= tegra124_io_pads_cfg_info,
> +	.num_cfg	= ARRAY_SIZE(tegra124_io_pads_cfg_info),
> +};
> +
> +static const struct tegra_io_pads_soc_data tegra210_io_pad_soc_data = {
> +	.desc		= tegra210_io_pads_pinctrl_desc,
> +	.num_desc	= ARRAY_SIZE(tegra210_io_pads_pinctrl_desc),
> +	.cfg		= tegra210_io_pads_cfg_info,
> +	.num_cfg	= ARRAY_SIZE(tegra210_io_pads_cfg_info),
> +};
> +
> +static const struct platform_device_id tegra_io_pads_dev_id[] = {
> +	{
> +		.name = "pinctrl-t124-io-pad",
> +		.driver_data = (kernel_ulong_t)&tegra124_io_pad_soc_data,
> +	}, {
> +		.name = "pinctrl-t210-io-pad",
> +		.driver_data = (kernel_ulong_t)&tegra210_io_pad_soc_data,
> +	}, {
> +	},
> +};
> +MODULE_DEVICE_TABLE(platform, tegra_io_pads_dev_id);
> +
> +static struct platform_driver tegra_io_pads_pinctrl_driver = {
> +	.driver		= {
> +		.name	= "pinctrl-tegra-io-pad",
> +	},
> +	.probe		= tegra_io_pads_pinctrl_probe,
> +	.id_table	= tegra_io_pads_dev_id,
> +};
> +
> +module_platform_driver(tegra_io_pads_pinctrl_driver);
> +
> +MODULE_DESCRIPTION("NVIDIA TEGRA IO pad Control Driver");
> +MODULE_AUTHOR("Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");

Like I said above, I think there's a lot of boilerplate in here that's
simply there to create a virtual device and bind a driver to it. All of
that comes with very little to no benefit. I think this could all just
be moved into the PMC driver and be simplified quite a bit.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH 04/10] power: supply: axp20x_usb_power: add 100mA max current limit for AXP223
From: Chen-Yu Tsai @ 2016-11-25  9:56 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Sebastian Reichel, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Russell King, Maxime Ripard, Lee Jones, open list:THERMAL,
	devicetree, linux-kernel, linux-arm-kernel, Thomas Petazzoni
In-Reply-To: <20161125090921.23138-5-quentin.schulz@free-electrons.com>

On Fri, Nov 25, 2016 at 5:09 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> The X-Powers AXP223 shares most of its behaviour with the AXP221 PMIC
> but allows the VBUS power supply max current to be set to 100mA (like
> the AXP209 PMIC).
>
> This basically adds a new compatible to the VBUS power supply driver and
> adds a check on the compatible when setting current max limit.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

^ permalink raw reply

* Re: [RFC PATCH net v2 0/3] Fix OdroidC2 Gigabit Tx link issue
From: Jerome Brunet @ 2016-11-25  9:55 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Florian Fainelli, Carlo Caione, Kevin Hilman, Giuseppe Cavallaro,
	Alexandre TORGUE, Andre Roth, Neil Armstrong,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAFBinCCg8cLuQdrRsxHA53JAvyWgfW7vfVMho0tj3bCCcu-YxA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, 2016-11-24 at 18:10 +0100, Martin Blumenstingl wrote:
> On Thu, Nov 24, 2016 at 5:01 PM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> wrote:
> > 
> > On Thu, 2016-11-24 at 15:40 +0100, Martin Blumenstingl wrote:
> > > 
> > > Hi Jerome,
> > > 
> > > On Mon, Nov 21, 2016 at 4:35 PM, Jerome Brunet <jbrunet@baylibre.
> > > com>
> > > wrote:
> > > > 
> > > > 
> > > > This patchset fixes an issue with the OdroidC2 board (DWMAC +
> > > > RTL8211F).
> > > > Initially reported as a low Tx throughput issue at gigabit
> > > > speed,
> > > > the
> > > > platform enters LPI too often. This eventually break the link
> > > > (both
> > > > Tx
> > > > and Rx), and require to bring the interface down and up again
> > > > to
> > > > get the
> > > > Rx path working again.
> > > > 
> > > > The root cause of this issue is not fully understood yet but
> > > > disabling EEE
> > > > advertisement on the PHY prevent this feature to be negotiated.
> > > > With this change, the link is stable and reliable, with the
> > > > expected
> > > > throughput performance.
> > > I have just sent a series which allows configuring the TX delay
> > > on
> > > the
> > > MAC (dwmac-meson8b glue) side: [0]
> > > Disabling the TX delay generated by the MAC fixes TX throughput
> > > for
> > > me, even when leaving EEE enabled in the RTL8211F PHY driver!
> > > 
> > > Unfortunately the RTL8211F PHY is a black-box for the community
> > > because there is no public datasheeet available.
> > > *maybe* (pure speculation!) they're enabling the TX delay based
> > > on
> > > some internal magic only when EEE is enabled.
> > 
> > Hi already tried acting on the register setting the TX_delay. I
> > also
> > tried on the PHY. I never been able to improve situation on the
> > Odroic2. Only disabling EEE improved the situation.
> OK, thanks for clarifying this!
> 
> > 
> > To make sure, i tried again with your patch but the result remains
> > unchanged. With Tx_delay disabled (either the mac or the phy), the
> > situation is even worse, it seems that nothing gets through
> This is interesting, because in your case you should have a 4ns TX
> delay (2ns from the MAC and presumably 2ns from the PHY).
> Maybe that is also the reason why the TX delay is configurable in 2ns
> steps in PRG_ETHERNET0 on Amlogic SoCs.
> 
> out of curiosity: have you tried setting a 4ns (half clock-cycle) TX
> delay for the MAC and disabling it in the PHY?

Just replied on the other thread. Long story short, Odroidc2 seems to
really require EEE to be switched off.

Again, thx for your help Martin

> 
> 
> Regards,
> Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: Jerome Brunet @ 2016-11-25  9:53 UTC (permalink / raw)
  To: Martin Blumenstingl, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
	mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
	carlo-KA+7E9HrN00dnm+yROfE0A
In-Reply-To: <CAFBinCB7sXjXor++W+PW0-j_VxATRzhexjqHgXj2jD10tBpZFg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, 2016-11-24 at 18:05 +0100, Martin Blumenstingl wrote:
> On Thu, Nov 24, 2016 at 4:56 PM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> wrote:
> > 
> > On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
> > > 
> > > Currently the dwmac-meson8b stmmac glue driver uses a hardcoded
> > > 1/4
> > > cycle TX clock delay. This seems to work fine for many boards
> > > (for
> > > example Odroid-C2 or Amlogic's reference boards) but there are
> > > some
> > > others where TX traffic is simply broken.
> > > There are probably multiple reasons why it's working on some
> > > boards
> > > while it's broken on others:
> > > - some of Amlogic's reference boards are using a Micrel PHY
> > > - hardware circuit design
> > > - maybe more...
> > > 
> > > This raises a question though:
> > > Which device is supposed to enable the TX delay when both MAC and
> > > PHY
> > > support it? And should we implement it for each PHY / MAC
> > > separately
> > > or should we think about a more generic solution (currently it's
> > > not
> > > possible to disable the TX delay generated by the RTL8211F PHY
> > > via
> > > devicetree when using phy-mode "rgmii")?
> > 
> > Actually you can skip the part which activate the Tx-delay on the
> > phy
> > by setting "phy-mode = "rgmii-id" instead of "rgmii"
> > 
> > phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
> > but PHY_INTERFACE_MODE_RGMII_ID.
> unfortunately this is not true for RTL8211F (I did my previous tests
> with the same expectation in mind)!
> the code seems to suggest that TX-delay is disabled whenever mode !=
> PHY_INTERFACE_MODE_RGMII.
> BUT: on my device RTL8211F_TX_DELAY is set even before
> "phy_write(phydev, 0x11, reg);"!

Thx a lot for pointing this out. I wrongly assumed that is was off by
default. It turns out it is ON on the OdroidC2 as well.

So I re-did all my test:
On the OdroidC2, as long as eee is not disabled (using ethtool or
through my patch) I can't get iperf to work properly. Disabling EEE,
the link works for the following configurations:

PHY TX Delay bit set : MAC delay 0ns, 2ns.
PHY TX Delay bit cleared : MAC delay 2ns, 4ns.

This seems to confirm that the PHY adds a 2ns delay when the bit is
set.

I could not test on my MXQPRO2.1 ... the PHY is no longer found during
the boot. Something died overnight :( 

> 
> Based on what I found it seems that rgmii-id, rgmii-txid and
> rgmii-rxid are supposed to be handled by the PHY.
> That would mean that we have two problems here:
> 1) drivers/net/phy/realtek.c:rtl8211f_config_init should check for
> PHY_INTERFACE_MODE_RGMII_ID or PHY_INTERFACE_MODE_RGMII_TXID and
> enable the TX-delay in that case - otherwise explicitly disable it
> 2) dwmac-meson8b.c should only use the configured TX-delay for
> PHY_INTERFACE_MODE_RGMII
> @Florian: could you please share your thoughts on this (who handles
> the TX delay in which case)?
> 
> 
> Regards,
> Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 02/10] power: supply: axp20x_usb_power: set min voltage and max current from sysfs
From: Thomas Petazzoni @ 2016-11-25  9:48 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: mark.rutland, devicetree, linux-pm, linux-kernel, sre, linux,
	wens, robh+dt, maxime.ripard, lee.jones, linux-arm-kernel
In-Reply-To: <20161125090921.23138-3-quentin.schulz@free-electrons.com>

Quentin,

On Fri, 25 Nov 2016 10:09:13 +0100, Quentin Schulz wrote:

> +static int axp20x_usb_power_set_property(struct power_supply *psy,
> +					 enum power_supply_property psp,
> +					 const union power_supply_propval *val)
> +{
> +	struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
> +	int ret, val1;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		switch (val->intval) {

This nested switch construct doesn't look very pretty. What about
instead:

	switch(psp) {
	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
		return axp20x_usb_power_set_prop_voltage_min(...);
	case POWER_SUPPLY_PROP_CURRENT_MAX:
		return axp20x_usb_power_set_prop_current_max(...);
	default:
		return -EINVAL;
	}

> +		case 4000000:
> +		case 4100000:
> +		case 4200000:
> +		case 4300000:
> +		case 4400000:
> +		case 4500000:
> +		case 4600000:
> +		case 4700000:
> +			val1 = (val->intval - 4000000) / 100000;
> +			ret = regmap_update_bits(power->regmap,
> +						 AXP20X_VBUS_IPSOUT_MGMT,
> +						 AXP20X_VBUS_VHOLD_MASK,
> +						 val1 << 3);
> +			if (ret)
> +				return ret;
> +
> +			return 0;

Just do:

			return regmap_update_bits(...);

The dance to test ret is useless, since you anyway return ret when
non-zero, or return zero when ret was zero.

While you're at it, maybe make val1 a u32, since I guess it's written
to a 32-bit register (unless u32 is not commonly used in this driver
already, of course).

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH 02/10] power: supply: axp20x_usb_power: set min voltage and max current from sysfs
From: Chen-Yu Tsai @ 2016-11-25  9:42 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Mark Rutland, devicetree, open list:THERMAL, linux-kernel,
	Sebastian Reichel, Russell King, Chen-Yu Tsai, Rob Herring,
	Maxime Ripard, Lee Jones, Thomas Petazzoni, linux-arm-kernel
In-Reply-To: <20161125090921.23138-3-quentin.schulz@free-electrons.com>

On Fri, Nov 25, 2016 at 5:09 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> AXP20X and AXP22X PMICs allow setting the min voltage and max current of
> VBUS power supply. This adds entries in sysfs to allow to do so.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
>  drivers/power/supply/axp20x_usb_power.c | 72 +++++++++++++++++++++++++++++++++
>  include/linux/mfd/axp20x.h              |  3 ++
>  2 files changed, 75 insertions(+)
>
> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
> index b19754e..638cb52 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
> @@ -155,6 +155,74 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
>         return 0;
>  }
>
> +static int axp20x_usb_power_set_property(struct power_supply *psy,
> +                                        enum power_supply_property psp,
> +                                        const union power_supply_propval *val)
> +{
> +       struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
> +       int ret, val1;
> +
> +       switch (psp) {
> +       case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +               switch (val->intval) {
> +               case 4000000:
> +               case 4100000:
> +               case 4200000:
> +               case 4300000:
> +               case 4400000:
> +               case 4500000:
> +               case 4600000:
> +               case 4700000:
> +                       val1 = (val->intval - 4000000) / 100000;
> +                       ret = regmap_update_bits(power->regmap,
> +                                                AXP20X_VBUS_IPSOUT_MGMT,
> +                                                AXP20X_VBUS_VHOLD_MASK,
> +                                                val1 << 3);

Please also define the shift offset.

> +                       if (ret)
> +                               return ret;
> +
> +                       return 0;
> +               default:
> +                       return -EINVAL;
> +               }
> +
> +               return 0;
> +
> +       case POWER_SUPPLY_PROP_CURRENT_MAX:
> +               switch (val->intval) {
> +               case 100000:
> +                       if (power->axp20x_id == AXP221_ID)
> +                               return -EINVAL;
> +               case 500000:
> +               case 900000:
> +                       val1 = (900000 - val->intval) / 400000;
> +                       ret = regmap_update_bits(power->regmap,
> +                                                AXP20X_VBUS_IPSOUT_MGMT,
> +                                                AXP20X_VBUS_CLIMIT_MASK, val1);
> +                       if (ret)
> +                               return ret;
> +
> +                       return 0;
> +               default:
> +                       return -EINVAL;
> +               }
> +
> +               return 0;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static int axp20x_usb_power_prop_writeable(struct power_supply *psy,
> +                                          enum power_supply_property psp)
> +{
> +       return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
> +              psp == POWER_SUPPLY_PROP_CURRENT_MAX;
> +}
> +
>  static enum power_supply_property axp20x_usb_power_properties[] = {
>         POWER_SUPPLY_PROP_HEALTH,
>         POWER_SUPPLY_PROP_PRESENT,
> @@ -178,7 +246,9 @@ static const struct power_supply_desc axp20x_usb_power_desc = {
>         .type = POWER_SUPPLY_TYPE_USB,
>         .properties = axp20x_usb_power_properties,
>         .num_properties = ARRAY_SIZE(axp20x_usb_power_properties),
> +       .property_is_writeable = axp20x_usb_power_prop_writeable,
>         .get_property = axp20x_usb_power_get_property,
> +       .set_property = axp20x_usb_power_set_property,
>  };
>
>  static const struct power_supply_desc axp22x_usb_power_desc = {
> @@ -186,7 +256,9 @@ static const struct power_supply_desc axp22x_usb_power_desc = {
>         .type = POWER_SUPPLY_TYPE_USB,
>         .properties = axp22x_usb_power_properties,
>         .num_properties = ARRAY_SIZE(axp22x_usb_power_properties),
> +       .property_is_writeable = axp20x_usb_power_prop_writeable,
>         .get_property = axp20x_usb_power_get_property,
> +       .set_property = axp20x_usb_power_set_property,
>  };
>
>  static const struct of_device_id axp20x_usb_power_match[] = {
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index fec597f..8883595 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -56,6 +56,9 @@ enum {
>  #define AXP20X_LDO24_V_OUT             0x28
>  #define AXP20X_LDO3_V_OUT              0x29
>  #define AXP20X_VBUS_IPSOUT_MGMT                0x30
> +
> +#define AXP20X_VBUS_VHOLD_MASK         GENMASK(5, 3)
> +

For the AXP drivers, we keep the per register bit field
definitions in the drivers that use them.

Regards
ChenYu

>  #define AXP20X_V_OFF                   0x31
>  #define AXP20X_OFF_CTRL                        0x32
>  #define AXP20X_CHRG_CTRL1              0x33
> --
> 2.9.3
>

^ permalink raw reply

* Re: [PATCH v6 3/5] ARM: dts: sun8i-h3: add HDMI video nodes
From: Icenowy Zheng @ 2016-11-25  9:41 UTC (permalink / raw)
  To: Jean-Francois Moine, Dave Airlie, Maxime Ripard, Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
In-Reply-To: <cdf50b8433e2d9a053e35f8788bfcd5d41504312.1479641523.git.moinejf-GANU6spQydw@public.gmane.org>



20.11.2016, 20:12, "Jean-Francois Moine" <moinejf@free.fr>:
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 51 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index 416b825..7c6b1d5 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -140,6 +140,16 @@
>                  #size-cells = <1>;
>                  ranges;
>
> + de: de-controller@01000000 {
> + compatible = "allwinner,sun8i-h3-display-engine";
> + reg = <0x01000000 0x400000>;
> + clocks = <&ccu CLK_BUS_DE>, <&ccu CLK_DE>;
> + clock-names = "bus", "clock";
> + resets = <&ccu RST_BUS_DE>;
> + ports = <&lcd0_p>;
> + status = "disabled";
> + };
> +
>                  dma: dma-controller@01c02000 {
>                          compatible = "allwinner,sun8i-h3-dma";
>                          reg = <0x01c02000 0x1000>;
> @@ -149,6 +159,23 @@
>                          #dma-cells = <1>;
>                  };
>
> + lcd0: lcd-controller@01c0c000 {
> + compatible = "allwinner,sun8i-a83t-tcon";
> + reg = <0x01c0c000 0x400>;
> + clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
> + clock-names = "bus", "clock";
> + resets = <&ccu RST_BUS_TCON0>;
> + interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + lcd0_p: port {
> + lcd0_hdmi: endpoint {
> + remote-endpoint = <&hdmi_lcd0>;
> + };
> + };
> + };
> +
>                  mmc0: mmc@01c0f000 {
>                          compatible = "allwinner,sun7i-a20-mmc";
>                          reg = <0x01c0f000 0x1000>;
> @@ -314,6 +341,11 @@
>                          clock-names = "hosc", "losc";
>                          #clock-cells = <1>;
>                          #reset-cells = <1>;
> +
> + assigned-clocks = <&ccu CLK_PLL_DE>,
> + <&ccu CLK_DE>;
> + assigned-clock-rates = <864000000>,
> + <432000000>;
>                  };
>
>                  pio: pinctrl@01c20800 {
> @@ -564,6 +596,25 @@
>                          interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>                  };
>
> + hdmi: hdmi@01ee0000 {
> + compatible = "allwinner,sun8i-h3-hdmi";
> + reg = <0x01ee0000 0x20000>;
> + clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
> + <&ccu CLK_HDMI_DDC>;
> + clock-names = "bus", "clock", "ddc-clock";
> + resets = <&ccu RST_BUS_HDMI0>, <&ccu RST_BUS_HDMI1>;
> + reset-names = "hdmi0", "hdmi1";
> + status = "disabled";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 { /* video */
> + reg = <0>;
> + hdmi_lcd0: endpoint {
> + remote-endpoint = <&lcd0_hdmi>;
> + };
> + };
> + };
> +
>                  rtc: rtc@01f00000 {
>                          compatible = "allwinner,sun6i-a31-rtc";
>                          reg = <0x01f00000 0x54>;

After removing CLK_PLL_DE's assigned-clock, the kernel passes compilation.

However, it cannot recognize any HDMI screen...

(My board is Orange Pi One, and I manually added status="okay"; to &lcd0, &de, &hdmi)

[   16.507802] sun8i-de2 1000000.de-controller: bound 1c0c000.lcd-controller (ops de2_lcd_ops [sun8i_de2_drm])
[   16.675948] sun8i-de2 1000000.de-controller: bound 1ee0000.hdmi (ops de2_hdmi_fini [sun8i_de2_hdmi])
[   16.685120] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[   16.695876] [drm] No driver support for vblank timestamp query.
[   16.701862] sun8i-de2 1000000.de-controller: No connectors reported connected with modes
[   16.713061] [drm] Cannot find any crtc or sizes - going 1024x768
[   16.734214] Console: switching to colour frame buffer device 128x48
[   16.751022] sun8i-de2 1000000.de-controller: fb0:  frame buffer device

> --
> 2.10.2
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH 03/10] Documentation: DT: binding: axp20x_usb_power: add axp223 compatible
From: Chen-Yu Tsai @ 2016-11-25  9:25 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Sebastian Reichel, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Russell King, Maxime Ripard, Lee Jones, open list:THERMAL,
	devicetree, linux-kernel, linux-arm-kernel, Thomas Petazzoni
In-Reply-To: <20161125090921.23138-4-quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

On Fri, Nov 25, 2016 at 5:09 PM, Quentin Schulz
<quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> This adds the "x-powers,axp223-usb-power-supply" to the list of
> compatibles for AXP20X VBUS power supply driver.

Please explain why we need this. Basically the part about
AXP221 vs AXP223 from your cover letter.

ChenYu

>
> Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt
> index f1d7bee..bf3953c 100644
> --- a/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt
> +++ b/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt
> @@ -3,6 +3,7 @@ AXP20x USB power supply
>  Required Properties:
>  -compatible: One of: "x-powers,axp202-usb-power-supply"
>                       "x-powers,axp221-usb-power-supply"
> +                     "x-powers,axp223-usb-power-supply"
>
>  This node is a subnode of the axp20x PMIC.
>
> --
> 2.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] dt-bindings: clarify compatible field usage for rockchip timers
From: Alexander Kochetkov @ 2016-11-25  9:23 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Caesar Wang, daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	huangtao-TNX95d0MmH7DzftRWevZcw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480025455-6797-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hello,

Here the thread where the patch was discussed:
http://lists.infradead.org/pipermail/linux-rockchip/2016-November/013174.html

Regards,
Alexander.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 01/10] power: supply: axp20x_usb_power: use of_device_id data field instead of device_is_compatible
From: Chen-Yu Tsai @ 2016-11-25  9:23 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Sebastian Reichel, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Russell King, Maxime Ripard, Lee Jones, open list:THERMAL,
	devicetree, linux-kernel, linux-arm-kernel, Thomas Petazzoni
In-Reply-To: <20161125090921.23138-2-quentin.schulz@free-electrons.com>

On Fri, Nov 25, 2016 at 5:09 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> This replaces calls to of_device_is_compatible to check data field of
> of_device_id matched when probing the driver.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
>  drivers/power/supply/axp20x_usb_power.c | 39 ++++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
> index 6af6feb..b19754e 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
> @@ -17,6 +17,7 @@
>  #include <linux/mfd/axp20x.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/power_supply.h>
>  #include <linux/regmap.h>
> @@ -45,6 +46,7 @@ struct axp20x_usb_power {
>         struct device_node *np;
>         struct regmap *regmap;
>         struct power_supply *supply;
> +       int axp20x_id;
>  };
>
>  static irqreturn_t axp20x_usb_power_irq(int irq, void *devid)
> @@ -86,8 +88,7 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
>
>                 switch (v & AXP20X_VBUS_CLIMIT_MASK) {
>                 case AXP20X_VBUC_CLIMIT_100mA:
> -                       if (of_device_is_compatible(power->np,
> -                                       "x-powers,axp202-usb-power-supply")) {
> +                       if (power->axp20x_id == AXP202_ID) {
>                                 val->intval = 100000;
>                         } else {
>                                 val->intval = -1; /* No 100mA limit */
> @@ -130,8 +131,7 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
>
>                 val->intval = POWER_SUPPLY_HEALTH_GOOD;
>
> -               if (of_device_is_compatible(power->np,
> -                               "x-powers,axp202-usb-power-supply")) {
> +               if (power->axp20x_id == AXP202_ID) {
>                         ret = regmap_read(power->regmap,
>                                           AXP20X_USB_OTG_STATUS, &v);
>                         if (ret)
> @@ -189,6 +189,17 @@ static const struct power_supply_desc axp22x_usb_power_desc = {
>         .get_property = axp20x_usb_power_get_property,
>  };
>
> +static const struct of_device_id axp20x_usb_power_match[] = {
> +       {
> +               .compatible = "x-powers,axp202-usb-power-supply",
> +               .data = (void *)AXP202_ID,
> +       }, {
> +               .compatible = "x-powers,axp221-usb-power-supply",
> +               .data = (void *)AXP221_ID,
> +       }, { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_usb_power_match);
> +

There's no need to move this. See why below.

>  static int axp20x_usb_power_probe(struct platform_device *pdev)
>  {
>         struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -200,11 +211,16 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>                 "VBUS_PLUGIN", "VBUS_REMOVAL", NULL };
>         static const char * const *irq_names;
>         const struct power_supply_desc *usb_power_desc;
> +       const struct of_device_id *of_id;
>         int i, irq, ret;
>
>         if (!of_device_is_available(pdev->dev.of_node))
>                 return -ENODEV;
>
> +       of_id = of_match_device(axp20x_usb_power_match, &pdev->dev);
> +       if (!of_id)
> +               return -ENODEV;
> +

You can use of_device_get_match_data() instead.

ChenYu

>         if (!axp20x) {
>                 dev_err(&pdev->dev, "Parent drvdata not set\n");
>                 return -EINVAL;
> @@ -214,11 +230,12 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>         if (!power)
>                 return -ENOMEM;
>
> +       power->axp20x_id = (int)of_id->data;
> +
>         power->np = pdev->dev.of_node;
>         power->regmap = axp20x->regmap;
>
> -       if (of_device_is_compatible(power->np,
> -                       "x-powers,axp202-usb-power-supply")) {
> +       if (power->axp20x_id == AXP202_ID) {
>                 /* Enable vbus valid checking */
>                 ret = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
>                                          AXP20X_VBUS_MON_VBUS_VALID,
> @@ -235,8 +252,7 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>
>                 usb_power_desc = &axp20x_usb_power_desc;
>                 irq_names = axp20x_irq_names;
> -       } else if (of_device_is_compatible(power->np,
> -                       "x-powers,axp221-usb-power-supply")) {
> +       } else if (power->axp20x_id == AXP221_ID) {
>                 usb_power_desc = &axp22x_usb_power_desc;
>                 irq_names = axp22x_irq_names;
>         } else {
> @@ -272,13 +288,6 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>         return 0;
>  }
>
> -static const struct of_device_id axp20x_usb_power_match[] = {
> -       { .compatible = "x-powers,axp202-usb-power-supply" },
> -       { .compatible = "x-powers,axp221-usb-power-supply" },
> -       { }
> -};
> -MODULE_DEVICE_TABLE(of, axp20x_usb_power_match);
> -
>  static struct platform_driver axp20x_usb_power_driver = {
>         .probe = axp20x_usb_power_probe,
>         .driver = {
> --
> 2.9.3
>

^ permalink raw reply

* Re: [PATCH V4 1/2] pinctrl: tegra: Add DT binding for io pads control
From: Thierry Reding @ 2016-11-25  9:13 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	swarren-3lzwWm7+Weoh9ZMKESR00Q, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	jonathanh-DDmLM1+adcrQT0dZR+AlfA, joe-6d6DIl74uiNBDgjK7y7TUQ,
	yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479976734-30498-2-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 8349 bytes --]

On Thu, Nov 24, 2016 at 02:08:53PM +0530, Laxman Dewangan wrote:
> NVIDIA Tegra124 and later SoCs support the multi-voltage level and
> low power state of some of its IO pads. The IO pads can work in
> the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
> sources. When IO interfaces are not used then IO pads can be
> configure in low power state to reduce the power consumption from
> that IO pads.
> 
> On Tegra124, the voltage level of IO power rail source is auto
> detected by hardware(SoC) and hence it is only require to configure
> in low power mode if IO pads are not used.
> 
> On T210 onwards, the auto-detection of voltage level from IO power

You say Tegra124 above but T210 here. Please use consistent spelling to
make it more obvious that we're talking about chip generations here.
Tegra<gen> is preferred over T<gen>.

> rail is removed from SoC and hence SW need to configure the PMC
> register explicitly to set proper voltage in IO pads based on
> IO rail power source voltage.
> 
> Add DT binding document for detailing the DT properties for
> configuring IO pads voltage levels and its power state.
> 
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> ---
> Changes from V1:
>  New in series based on pinctrl driver requirement.
> 
> Changes from V2:
>  Updated the statement to say 1.8V and 3.3V as nominal voltage.
>  Corrected DT example by adding -supply and taken care of V1 review
>  from Rob.
> 
>  .../bindings/pinctrl/nvidia,tegra-io-pad.txt       | 126 +++++++++++++++++++++
>  1 file changed, 126 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> new file mode 100644
> index 0000000..a88c484
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> @@ -0,0 +1,126 @@
> +NVIDIA Tegra PMC IO pad controller
> +
> +NVIDIA Tegra124 and later SoCs support the multi-voltage level and low power
> +state of some of its IO pads. When IO interface are not used then IO pads can
> +be configure in low power state to reduce the power from that IO pads. The IO
> +pads can work in the nominal IO voltage of 1.8V and 3.3V from power rail
> +sources.
> +
> +On Tegra124, the voltage of IO power rail source is auto detected by SoC and
> +hence it is only require to configure in low power mode if IO pads are not
> +used.
> +
> +On T210 onwards, the HW based auto-detection for IO voltage is removed and
> +hence SW need to configure the PMC register explicitly, to set proper voltage
> +in IO pads, based on IO rail power source voltage.
> +
> +The voltage configurations and low power state of IO pads should be done in
> +boot if it is not going to change otherwise dynamically based on IO rail
> +voltage on that IO pads and usage of IO pads

Missing fullstop at the end.

> +
> +The DT property of the IO pads must be under the node of pmc i.e.
> +pmc@7000e400 for Tegra124 onwards.

The PMC is at a different address on Tegra186, so I think we should just
drop this to avoid having to update it whenever a new chip relocates it
to a different address.

Come to think of it, if these I/O pads are represented as subnodes in
the PMC device tree node, perhaps this should be merged into the PMC
documentation?

> +Please refer to <pinctrl-bindings.txt> in this directory for details of the
> +common pinctrl bindings used by client devices, including the meaning of the
> +phrase "pin configuration node".
> +
> +Tegra's pin configuration nodes act as a container for an arbitrary number of
> +subnodes. Each of these subnodes represents some desired configuration for an
> +IO pads, or a list of IO pads. This configuration can include the voltage and
> +power enable/disable control

Missing fullstop.

> +
> +The name of each subnode is not important; all subnodes should be enumerated
> +and processed purely based on their content. Each subnode only affects those
> +parameters that are explicitly listed. Unspecified is represented as an absent
> +property,

Should be fullstop instead of comma at the end of the sentence. Also I'm
not sure the last sentence is even necessary. The previous one already
says that only explicitly listed parameters take effect.

> +
> +See the TRM to determine which properties and values apply to each IO pads.

Perhaps give a reference to where in the TRM this can be found?

> +
> +Required subnode-properties:
> +==========================
> +- pins : An array of strings. Each string contains the name of an IO pads. Valid
> +	 values for these names are listed below.
> +
> +Optional subnode-properties:
> +==========================
> +Following properties are supported from generic pin configuration explained
> +in <dt-bindings/pinctrl/pinctrl-binding.txt>.

I don't think such a directory exists. Maybe make these relative
references, though that might become slightly more difficult if this
whole document is merged with the PMC documentation.

> +low-power-enable:		enable low power mode.
> +low-power-disable:		disable low power mode.

Why the extra padding with tabs? I find that difficult to read. Also, no
need for a fullstop since it's not a proper sentence.

> +Valid values for pin for T124 are:

Tegra124

> +	audio, bb, cam, comp, csia, csib, csie, dsi, dsib, dsic, dsid, hdmi,
> +	hsic, hv, lvds, mipi-bias, nand, pex-bias, pex-clk1, pex-clk2,
> +	pex-ctrl, sdmmc1, sdmmc3, sdmmc4, sys-ddc, uart, usb0, usb1, usb2,
> +	usb-bias
> +
> +Valid values for pin for T210 are:

Tegra210

> +	audio, audio-hv, cam, csia, csib, csic, csid, csie, csif,
> +	dbg, debug-nonao, dmic, dp, dsi, dsib, dsic, dsid, emmc, emmc2,
> +	gpio, hdmi, hsic, lvds, mipi-bias, pex-bias, pex-clk1, pex-clk2,
> +	pex-ctrl, sdmmc1, sdmmc3, spi, spi-hv, uart, usb-bias, usb0,
> +	usb1, usb2, usb3.
> +
> +To find out the IO rail voltage for setting the voltage of IO pad by SW,
> +the regulator supply handle must provided from the DT and it is explained
> +in the regulator DT binding document
> +	<devicetree/bindings/regulator/regulator.txt>.

Again, maybe make this relative because as it is it isn't obvious as to
what the base of the above path is.

> +For example, for GPIO rail the supply name is vddio-gpio and regulator
> +handle is supplied from DT as
> +	vddio-gpio-supply = <&regulator_xyz>;
> +
> +For T210, following IO pads support the 1.8V/3.3V and the corresponding

Tegra210

> +IO voltage pin names are as follows:
> +	audio -> vddio-audio
> +	audio-hv -> vddio-audio-hv
> +	cam ->vddio-cam
> +	dbg -> vddio-dbg
> +	dmic -> vddio-dmic
> +	gpio -> vddio-gpio
> +	pex-ctrl -> vddio-pex-ctrl
> +	sdmmc1 -> vddio-sdmmc1
> +	sdmmc3 -> vddio-sdmmc3
> +	spi -> vddio-spi
> +	spi-hv -> vddio-spi-hv
> +	uart -> vddio-uart

It's slightly confusing to only have this list for Tegra210. I assume
that is because on Tegra124 there is no way to control the voltage of
the pins, but I think that could be made clearer. Also, it might be
worth explicitly mentioning that this is a subset of the list of pins
given above and that the other pins (those not in this list) don't
support 1.8/3.3 V control, but only the low power state.

> +
> +Example:
> +	i2c@7000d000 {
> +		pmic@3c {
> +			regulators {
> +				vddio_sdmmc1: ldo2 {
> +					/* Regulator entries for LDO2 */
> +				};
> +
> +				vdd_cam: ldo3 {
> +					/* Regulator entries for LDO3 */

These comments are somewhat stating the obvious. Perhaps simply use a
... as a placeholder?

> +				};
> +			};
> +		};
> +	};
> +
> +	pmc@7000e400 {
> +		vddio-cam-supply = <&vdd_cam>;
> +		vddio-sdmmc1-supply = <&vddio_sdmmc1>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&tegra_io_pad_volt_default>;
> +		tegra_io_pad_volt_default: common {
> +			audio-hv {
> +				pins = "audio-hv";
> +				low-power-disable;
> +			};

I wonder if this is at all useful. Shouldn't we rather put all pads into
a low-power state by default and only take them out of the low-power
state when the driver decides to do so?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* [PATCH] ARM: davinci_all_defconfig: enable dumb vga-dac drm bridge
From: Bartosz Golaszewski @ 2016-11-25  9:13 UTC (permalink / raw)
  To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
	Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
  Cc: linux-devicetree, LKML, linux-drm, Bartosz Golaszewski,
	Tomi Valkeinen, Jyri Sarha, arm-soc, Laurent Pinchart

This enables the dumb-vga-dac driver by default for davinci boards.

The driver is needed for tilcdc support on da850-lcdk board.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/configs/davinci_all_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
index b5e978f..ab1bf18 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -127,6 +127,8 @@ CONFIG_REGULATOR_FIXED_VOLTAGE=y
 CONFIG_REGULATOR_TPS6507X=y
 CONFIG_DRM=m
 CONFIG_DRM_TILCDC=m
+CONFIG_DRM_BRIDGE=y
+CONFIG_DRM_DUMB_VGA_DAC=m
 CONFIG_FB=y
 CONFIG_FIRMWARE_EDID=y
 CONFIG_FB_DA8XX=y
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [PATCH] ARM: da850-lcdk: add the dumb-vga-dac node
From: Bartosz Golaszewski @ 2016-11-25  9:11 UTC (permalink / raw)
  To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
	Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
  Cc: linux-devicetree, LKML, linux-drm, Bartosz Golaszewski,
	Tomi Valkeinen, Jyri Sarha, arm-soc, Laurent Pinchart

Add the dumb-vga-dac node to the board DT together with corresponding
ports and vga connector. This allows to retrieve the edid info from
the display automatically.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/boot/dts/da850-lcdk.dts | 58 ++++++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/da850.dtsi     | 17 ++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21..c5d3a2b 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -50,6 +50,53 @@
 			system-clock-frequency = <24576000>;
 		};
 	};
+
+	vga_bridge {
+		compatible = "dumb-vga-dac";
+		pinctrl-names = "default";
+		pinctrl-0 = <&lcd_pins>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+
+				vga_bridge_in: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&display_out_vga>;
+				};
+			};
+
+			port@1 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <1>;
+
+				vga_bridge_out: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&vga_con_in>;
+				};
+			};
+		};
+	};
+
+	vga {
+		compatible = "vga-connector";
+
+		ddc-i2c-bus = <&i2c0>;
+
+		port {
+			vga_con_in: endpoint {
+				remote-endpoint = <&vga_bridge_out>;
+			};
+		};
+	};
 };
 
 &pmx_core {
@@ -219,3 +266,14 @@
 		};
 	};
 };
+
+&display {
+	status = "okay";
+};
+
+&display_out {
+	display_out_vga: endpoint@0 {
+		reg = <0>;
+		remote-endpoint = <&vga_bridge_in>;
+	};
+};
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 36066fa..6ab8df1 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -442,6 +442,23 @@
 			reg = <0x213000 0x1000>;
 			interrupts = <52>;
 			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				display_in: port@0 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0>;
+				};
+
+				display_out: port@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+				};
+			};
 		};
 	};
 	aemif: aemif@68000000 {
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [PATCH 10/10] ARM: dtsi: sun8i-reference-design-tablet: use AXP223 DTSI
From: Quentin Schulz @ 2016-11-25  9:09 UTC (permalink / raw)
  To: sre-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, wens-jdAy2FN1RRM,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A
  Cc: Quentin Schulz, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
In-Reply-To: <20161125090921.23138-1-quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Previously, the sun8i tablets used everything declared in AXP221 DTSI
while they have an AXP223 PMIC.

This corrects that so the sun8i tablets can get some features the AXP223
has (at the moment, ability to have 100mA as maximal current on VBUS
power supply).

Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi b/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
index 08cd001..ea79c33 100644
--- a/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
+++ b/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
@@ -136,7 +136,7 @@
 	};
 };
 
-#include "axp22x.dtsi"
+#include "axp223.dtsi"
 
 &reg_aldo1 {
 	regulator-always-on;
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 09/10] ARM: dts: sun8i-r16-parrot: use AXP223 DTSI
From: Quentin Schulz @ 2016-11-25  9:09 UTC (permalink / raw)
  To: sre, robh+dt, mark.rutland, wens, linux, maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, thomas.petazzoni
In-Reply-To: <20161125090921.23138-1-quentin.schulz@free-electrons.com>

Previously, the Allwinner Parrot R16 used everything declared in AXP221
DTSI while it has an AXP223 PMIC.

This corrects that so the Allwinner Parrot R16 can get some features the
AXP223 has (at the moment, ability to have 100mA as maximal current on
VBUS power supply).

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-r16-parrot.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-r16-parrot.dts b/arch/arm/boot/dts/sun8i-r16-parrot.dts
index 47553e5..6afdba3 100644
--- a/arch/arm/boot/dts/sun8i-r16-parrot.dts
+++ b/arch/arm/boot/dts/sun8i-r16-parrot.dts
@@ -209,7 +209,7 @@
 	};
 };
 
-#include "axp22x.dtsi"
+#include "axp223.dtsi"
 
 &reg_aldo1 {
 	regulator-always-on;
-- 
2.9.3


^ permalink raw reply related

* [PATCH 08/10] ARM: dts: sun8i-a33-sinlinx-sina33: use AXP223 DTSI
From: Quentin Schulz @ 2016-11-25  9:09 UTC (permalink / raw)
  To: sre, robh+dt, mark.rutland, wens, linux, maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, thomas.petazzoni
In-Reply-To: <20161125090921.23138-1-quentin.schulz@free-electrons.com>

Previously, the Sinlinx SinA33 used everything declared in AXP221 DTSI
while it has an AXP223 PMIC.

This corrects that so the Sinlinx SinA33 can get some features the
AXP223 has (at the moment, ability to have 100mA as maximal current on
VBUS power supply).

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts b/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
index 573aa2c..3373164 100644
--- a/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
+++ b/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
@@ -161,7 +161,7 @@
 	};
 };
 
-#include "axp22x.dtsi"
+#include "axp223.dtsi"
 
 &reg_aldo1 {
 	regulator-always-on;
-- 
2.9.3


^ permalink raw reply related

* [PATCH 07/10] ARM: dts: sun8i-a33-olinuxino: use AXP223 DTSI
From: Quentin Schulz @ 2016-11-25  9:09 UTC (permalink / raw)
  To: sre, robh+dt, mark.rutland, wens, linux, maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, thomas.petazzoni
In-Reply-To: <20161125090921.23138-1-quentin.schulz@free-electrons.com>

Previously, the Olimex A33-OlinuXino used everything declared in AXP221
DTSI while it has an AXP223 PMIC.

This corrects that so the Olimex A33-OlinuXino can get some features the
AXP223 has (at the moment, ability to have 100mA as maximal current on
VBUS power supply).

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a33-olinuxino.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-a33-olinuxino.dts b/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
index 9ea637e..3e8f2ec 100644
--- a/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
+++ b/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
@@ -126,7 +126,7 @@
 	};
 };
 
-#include "axp22x.dtsi"
+#include "axp223.dtsi"
 
 &reg_aldo1 {
 	regulator-always-on;
-- 
2.9.3


^ permalink raw reply related

* [PATCH 06/10] ARM: dtsi: add DTSI for AXP223
From: Quentin Schulz @ 2016-11-25  9:09 UTC (permalink / raw)
  To: sre-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, wens-jdAy2FN1RRM,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A
  Cc: Quentin Schulz, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
In-Reply-To: <20161125090921.23138-1-quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The AXP223 shares most of its logic with the AXP221 but it has some
differences for the VBUS driver.

Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/axp223.dtsi | 55 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 arch/arm/boot/dts/axp223.dtsi

diff --git a/arch/arm/boot/dts/axp223.dtsi b/arch/arm/boot/dts/axp223.dtsi
new file mode 100644
index 0000000..f8ce55c
--- /dev/null
+++ b/arch/arm/boot/dts/axp223.dtsi
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2016 Free Electrons
+ *
+ * Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/*
+ * AXP223 Integrated Power Management Chip
+ * http://www.x-powers.com/product/AXP22X.php
+ * http://dl.linux-sunxi.org/AXP/AXP223-en.pdf
+ */
+
+#include "axp22x.dtsi"
+
+&usb_power_supply {
+	compatible = "x-powers,axp223-usb-power-supply";
+};
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 05/10] mfd: axp20x: add separate MFD cell for AXP223
From: Quentin Schulz @ 2016-11-25  9:09 UTC (permalink / raw)
  To: sre, robh+dt, mark.rutland, wens, linux, maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, thomas.petazzoni
In-Reply-To: <20161125090921.23138-1-quentin.schulz@free-electrons.com>

The AXP223 shares most of its logic with the AXP221 but has some
differences for the VBUS power supply driver. Thus, to probe the driver
with the correct compatible, the AXP221 and the AXP223 now have separate
MFD cells.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 drivers/mfd/axp20x.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index ba130be..989d568 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -602,6 +602,21 @@ static struct mfd_cell axp22x_cells[] = {
 	},
 };
 
+static struct mfd_cell axp223_cells[] = {
+	{
+		.name			= "axp20x-pek",
+		.num_resources		= ARRAY_SIZE(axp22x_pek_resources),
+		.resources		= axp22x_pek_resources,
+	}, {
+		.name			= "axp20x-regulator",
+	}, {
+		.name		= "axp20x-usb-power-supply",
+		.of_compatible	= "x-powers,axp223-usb-power-supply",
+		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
+		.resources	= axp22x_usb_power_supply_resources,
+	},
+};
+
 static struct mfd_cell axp152_cells[] = {
 	{
 		.name			= "axp20x-pek",
@@ -789,12 +804,17 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
 		axp20x->regmap_irq_chip = &axp20x_regmap_irq_chip;
 		break;
 	case AXP221_ID:
-	case AXP223_ID:
 		axp20x->nr_cells = ARRAY_SIZE(axp22x_cells);
 		axp20x->cells = axp22x_cells;
 		axp20x->regmap_cfg = &axp22x_regmap_config;
 		axp20x->regmap_irq_chip = &axp22x_regmap_irq_chip;
 		break;
+	case AXP223_ID:
+		axp20x->nr_cells = ARRAY_SIZE(axp223_cells);
+		axp20x->cells = axp223_cells;
+		axp20x->regmap_cfg = &axp22x_regmap_config;
+		axp20x->regmap_irq_chip = &axp22x_regmap_irq_chip;
+		break;
 	case AXP288_ID:
 		axp20x->cells = axp288_cells;
 		axp20x->nr_cells = ARRAY_SIZE(axp288_cells);
-- 
2.9.3


^ permalink raw reply related

* [PATCH 04/10] power: supply: axp20x_usb_power: add 100mA max current limit for AXP223
From: Quentin Schulz @ 2016-11-25  9:09 UTC (permalink / raw)
  To: sre, robh+dt, mark.rutland, wens, linux, maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, thomas.petazzoni
In-Reply-To: <20161125090921.23138-1-quentin.schulz@free-electrons.com>

The X-Powers AXP223 shares most of its behaviour with the AXP221 PMIC
but allows the VBUS power supply max current to be set to 100mA (like
the AXP209 PMIC).

This basically adds a new compatible to the VBUS power supply driver and
adds a check on the compatible when setting current max limit.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 drivers/power/supply/axp20x_usb_power.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
index 638cb52..6d5d451 100644
--- a/drivers/power/supply/axp20x_usb_power.c
+++ b/drivers/power/supply/axp20x_usb_power.c
@@ -88,11 +88,10 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
 
 		switch (v & AXP20X_VBUS_CLIMIT_MASK) {
 		case AXP20X_VBUC_CLIMIT_100mA:
-			if (power->axp20x_id == AXP202_ID) {
-				val->intval = 100000;
-			} else {
+			if (power->axp20x_id == AXP221_ID)
 				val->intval = -1; /* No 100mA limit */
-			}
+			else
+				val->intval = 100000;
 			break;
 		case AXP20X_VBUC_CLIMIT_500mA:
 			val->intval = 500000;
@@ -268,6 +267,9 @@ static const struct of_device_id axp20x_usb_power_match[] = {
 	}, {
 		.compatible = "x-powers,axp221-usb-power-supply",
 		.data = (void *)AXP221_ID,
+	}, {
+		.compatible = "x-powers,axp223-usb-power-supply",
+		.data = (void *)AXP223_ID,
 	}, { /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, axp20x_usb_power_match);
@@ -324,7 +326,8 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
 
 		usb_power_desc = &axp20x_usb_power_desc;
 		irq_names = axp20x_irq_names;
-	} else if (power->axp20x_id == AXP221_ID) {
+	} else if (power->axp20x_id == AXP221_ID ||
+		   power->axp20x_id == AXP223_ID) {
 		usb_power_desc = &axp22x_usb_power_desc;
 		irq_names = axp22x_irq_names;
 	} else {
-- 
2.9.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox