Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 2/6] thermal: amlogic: Add thermal driver to support G12 SoCs
From: Guillaume La Roque @ 2019-08-21 22:24 UTC (permalink / raw)
  To: rui.zhang, edubezval, daniel.lezcano
  Cc: devicetree, linux-amlogic, linux-kernel, linux-arm-kernel,
	linux-pm
In-Reply-To: <20190821222421.30242-1-glaroque@baylibre.com>

Amlogic G12A and G12B SoCs integrate two thermal sensors
with the same design.
One is located close to the DDR controller and the other one is
located close to the PLLs (between the CPU and GPU).

The calibration data for each of the thermal sensors instance is
stored in a different location within the AO region.

Implement reading the temperature from each thermal sensor.

The IP block has more functionality, which may be added to this driver
in the future:
- chip reset when the temperature exceeds a configurable threshold
- up to four interrupts when the temperature has risen above a
configurable threshold
- up to four interrupts when the temperature has fallen below a
configurable threshold

Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
 drivers/thermal/Kconfig           |  11 +
 drivers/thermal/Makefile          |   1 +
 drivers/thermal/amlogic_thermal.c | 335 ++++++++++++++++++++++++++++++
 3 files changed, 347 insertions(+)
 create mode 100644 drivers/thermal/amlogic_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 9966364a6deb..0f31bb4bc372 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -348,6 +348,17 @@ config MTK_THERMAL
 	  Enable this option if you want to have support for thermal management
 	  controller present in Mediatek SoCs
 
+config AMLOGIC_THERMAL
+	tristate "Amlogic Thermal Support"
+	default ARCH_MESON
+	depends on OF && ARCH_MESON
+	help
+	  If you say yes here you get support for Amlogic Thermal
+	  for G12 SoC Family.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called amlogic_thermal.
+
 menu "Intel thermal drivers"
 depends on X86 || X86_INTEL_QUARK || COMPILE_TEST
 source "drivers/thermal/intel/Kconfig"
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 74a37c7f847a..baeb70bf0568 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -54,3 +54,4 @@ obj-$(CONFIG_MTK_THERMAL)	+= mtk_thermal.o
 obj-$(CONFIG_GENERIC_ADC_THERMAL)	+= thermal-generic-adc.o
 obj-$(CONFIG_ZX2967_THERMAL)	+= zx2967_thermal.o
 obj-$(CONFIG_UNIPHIER_THERMAL)	+= uniphier_thermal.o
+obj-$(CONFIG_AMLOGIC_THERMAL)     += amlogic_thermal.o
diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c
new file mode 100644
index 000000000000..173d808048d2
--- /dev/null
+++ b/drivers/thermal/amlogic_thermal.c
@@ -0,0 +1,335 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Thermal Sensor Driver
+ *
+ * Copyright (C) 2017 Huan Biao <huan.biao@amlogic.com>
+ * Copyright (C) 2019 Guillaume La Roque <glaroque@baylibre.com>
+ *
+ * Register value to celsius temperature formulas:
+ *	Read_Val	    m * U
+ * U = ---------, Uptat = ---------
+ *	2^16		  1 + n * U
+ *
+ * Temperature = A * ( Uptat + u_efuse / 2^16 )- B
+ *
+ *  A B m n : calibration parameters
+ *  u_efuse : fused calibration value, it's a signed 16 bits value
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+#define TSENSOR_CFG_REG1			0x4
+	#define TSENSOR_CFG_REG1_RSET_VBG	BIT(12)
+	#define TSENSOR_CFG_REG1_RSET_ADC	BIT(11)
+	#define TSENSOR_CFG_REG1_VCM_EN		BIT(10)
+	#define TSENSOR_CFG_REG1_VBG_EN		BIT(9)
+	#define TSENSOR_CFG_REG1_OUT_CTL	BIT(6)
+	#define TSENSOR_CFG_REG1_FILTER_EN	BIT(5)
+	#define TSENSOR_CFG_REG1_DEM_EN		BIT(3)
+	#define TSENSOR_CFG_REG1_CH_SEL		GENMASK(1, 0)
+	#define TSENSOR_CFG_REG1_ENABLE		\
+		(TSENSOR_CFG_REG1_FILTER_EN |	\
+		 TSENSOR_CFG_REG1_VCM_EN |	\
+		 TSENSOR_CFG_REG1_VBG_EN |	\
+		 TSENSOR_CFG_REG1_DEM_EN |	\
+		 TSENSOR_CFG_REG1_CH_SEL)
+
+#define TSENSOR_STAT0			0x40
+
+#define TSENSOR_STAT9			0x64
+
+#define TSENSOR_READ_TEMP_MASK		GENMASK(15, 0)
+#define TSENSOR_TEMP_MASK		GENMASK(11, 0)
+
+#define TSENSOR_TRIM_SIGN_MASK		BIT(15)
+#define TSENSOR_TRIM_TEMP_MASK		GENMASK(14, 0)
+#define TSENSOR_TRIM_VERSION_MASK	GENMASK(31, 24)
+
+#define TSENSOR_TRIM_VERSION(_version)	\
+	FIELD_GET(TSENSOR_TRIM_VERSION_MASK, _version)
+
+#define TSENSOR_TRIM_CALIB_VALID_MASK	(GENMASK(3, 2) | BIT(7))
+
+#define TSENSOR_CALIB_OFFSET	1
+#define TSENSOR_CALIB_SHIFT	4
+
+/**
+ * struct amlogic_thermal_soc_calib_data
+ * @A, B, m, n: calibration parameters
+ * This structure is required for configuration of amlogic thermal driver.
+ */
+struct amlogic_thermal_soc_calib_data {
+	int A;
+	int B;
+	int m;
+	int n;
+};
+
+/**
+ * struct amlogic_thermal_data
+ * @u_efuse_off: register offset to read fused calibration value
+ * @soc: calibration parameters structure pointer
+ * @regmap_config: regmap config for the device
+ * This structure is required for configuration of amlogic thermal driver.
+ */
+struct amlogic_thermal_data {
+	int u_efuse_off;
+	const struct amlogic_thermal_soc_calib_data *calibration_parameters;
+	const struct regmap_config *regmap_config;
+};
+
+struct amlogic_thermal {
+	struct platform_device *pdev;
+	const struct amlogic_thermal_data *data;
+	struct regmap *regmap;
+	struct regmap *sec_ao_map;
+	struct clk *clk;
+	struct thermal_zone_device *tzd;
+	u32 trim_info;
+};
+
+/*
+ * Calculate a temperature value from a temperature code.
+ * The unit of the temperature is degree milliCelsius.
+ */
+static int amlogic_thermal_code_to_millicelsius(struct amlogic_thermal *pdata,
+						int temp_code)
+{
+	const struct amlogic_thermal_soc_calib_data *param =
+					pdata->data->calibration_parameters;
+	int temp;
+	s64 factor, Uptat, uefuse;
+
+	uefuse = pdata->trim_info & TSENSOR_TRIM_SIGN_MASK ?
+			     ~(pdata->trim_info & TSENSOR_TRIM_TEMP_MASK) + 1 :
+			     (pdata->trim_info & TSENSOR_TRIM_TEMP_MASK);
+
+	factor = param->n * temp_code;
+	factor = div_s64(factor, 100);
+
+	Uptat = temp_code * param->m;
+	Uptat = div_s64(Uptat, 100);
+	Uptat = Uptat * BIT(16);
+	Uptat = div_s64(Uptat, BIT(16) + factor);
+
+	temp = (Uptat + uefuse) * param->A;
+	temp = div_s64(temp, BIT(16));
+	temp = (temp - param->B) * 100;
+
+	return temp;
+}
+
+static int amlogic_thermal_initialize(struct amlogic_thermal *pdata)
+{
+	int ret = 0;
+	int ver;
+
+	regmap_read(pdata->sec_ao_map, pdata->data->u_efuse_off,
+		    &pdata->trim_info);
+
+	ver = TSENSOR_TRIM_VERSION(pdata->trim_info);
+
+	if ((ver & TSENSOR_TRIM_CALIB_VALID_MASK) == 0) {
+		ret = -EINVAL;
+		dev_err(&pdata->pdev->dev,
+			"tsensor thermal calibration not supported: 0x%x!\n",
+			ver);
+	}
+
+	return ret;
+}
+
+static int amlogic_thermal_enable(struct amlogic_thermal *data)
+{
+	int ret;
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret)
+		return ret;
+
+	regmap_update_bits(data->regmap, TSENSOR_CFG_REG1,
+			   TSENSOR_CFG_REG1_ENABLE, TSENSOR_CFG_REG1_ENABLE);
+
+	return 0;
+}
+
+static int amlogic_thermal_disable(struct amlogic_thermal *data)
+{
+	regmap_update_bits(data->regmap, TSENSOR_CFG_REG1,
+			   TSENSOR_CFG_REG1_ENABLE, 0);
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+static int amlogic_thermal_get_temp(void *data, int *temp)
+{
+	unsigned int tval;
+	struct amlogic_thermal *pdata = data;
+
+	if (!data)
+		return -EINVAL;
+
+	regmap_read(pdata->regmap, TSENSOR_STAT0, &tval);
+	*temp =
+	   amlogic_thermal_code_to_millicelsius(pdata,
+						tval & TSENSOR_READ_TEMP_MASK);
+
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops amlogic_thermal_ops = {
+	.get_temp	= amlogic_thermal_get_temp,
+};
+
+static const struct regmap_config amlogic_thermal_regmap_config_g12a = {
+	.reg_bits = 8,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = TSENSOR_STAT9,
+};
+
+static const struct amlogic_thermal_soc_calib_data amlogic_thermal_g12a = {
+	.A = 9411,
+	.B = 3159,
+	.m = 424,
+	.n = 324,
+};
+
+static const struct amlogic_thermal_data amlogic_thermal_g12a_cpu_param = {
+	.u_efuse_off = 0x128,
+	.calibration_parameters = &amlogic_thermal_g12a,
+	.regmap_config = &amlogic_thermal_regmap_config_g12a,
+};
+
+static const struct amlogic_thermal_data amlogic_thermal_g12a_ddr_param = {
+	.u_efuse_off = 0xf0,
+	.calibration_parameters = &amlogic_thermal_g12a,
+	.regmap_config = &amlogic_thermal_regmap_config_g12a,
+};
+
+static const struct of_device_id of_amlogic_thermal_match[] = {
+	{
+		.compatible = "amlogic,g12a-ddr-thermal",
+		.data = &amlogic_thermal_g12a_ddr_param,
+	},
+	{
+		.compatible = "amlogic,g12a-cpu-thermal",
+		.data = &amlogic_thermal_g12a_cpu_param,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_amlogic_thermal_match);
+
+static int amlogic_thermal_probe(struct platform_device *pdev)
+{
+	struct amlogic_thermal *pdata;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	void __iomem *base;
+	int ret;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	pdata->data = of_device_get_match_data(dev);
+	pdata->pdev = pdev;
+	platform_set_drvdata(pdev, pdata);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base)) {
+		dev_err(dev, "failed to get io address\n");
+		return PTR_ERR(base);
+	}
+
+	pdata->regmap = devm_regmap_init_mmio(dev, base,
+					      pdata->data->regmap_config);
+	if (IS_ERR(pdata->regmap))
+		return PTR_ERR(pdata->regmap);
+
+	pdata->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(pdata->clk)) {
+		if (PTR_ERR(pdata->clk) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get clock\n");
+		return PTR_ERR(pdata->clk);
+	}
+
+	pdata->sec_ao_map = syscon_regmap_lookup_by_phandle
+		(pdev->dev.of_node, "amlogic,ao-secure");
+	if (IS_ERR(pdata->sec_ao_map)) {
+		dev_err(dev, "syscon regmap lookup failed.\n");
+		return PTR_ERR(pdata->sec_ao_map);
+	}
+
+	pdata->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
+							  0,
+							  pdata,
+							  &amlogic_thermal_ops);
+	if (IS_ERR(pdata->tzd)) {
+		ret = PTR_ERR(pdata->tzd);
+		dev_err(dev, "Failed to register tsensor: %d\n", ret);
+		return PTR_ERR(pdata->tzd);
+	}
+
+	ret = amlogic_thermal_initialize(pdata);
+	if (ret)
+		return ret;
+
+	ret = amlogic_thermal_enable(pdata);
+
+	return ret;
+}
+
+static int amlogic_thermal_remove(struct platform_device *pdev)
+{
+	struct amlogic_thermal *data = platform_get_drvdata(pdev);
+
+	return amlogic_thermal_disable(data);
+}
+
+static int __maybe_unused amlogic_thermal_suspend(struct device *dev)
+{
+	struct amlogic_thermal *data = dev_get_drvdata(dev);
+
+	return amlogic_thermal_disable(data);
+}
+
+static int __maybe_unused amlogic_thermal_resume(struct device *dev)
+{
+	struct amlogic_thermal *data = dev_get_drvdata(dev);
+
+	return amlogic_thermal_enable(data);
+}
+
+static SIMPLE_DEV_PM_OPS(amlogic_thermal_pm_ops,
+			 amlogic_thermal_suspend, amlogic_thermal_resume);
+
+static struct platform_driver amlogic_thermal_driver = {
+	.driver = {
+		.name		= "amlogic_thermal",
+		.pm		= &amlogic_thermal_pm_ops,
+		.of_match_table = of_amlogic_thermal_match,
+	},
+	.probe	= amlogic_thermal_probe,
+	.remove	= amlogic_thermal_remove,
+};
+
+module_platform_driver(amlogic_thermal_driver);
+
+MODULE_AUTHOR("Guillaume La Roque <glaroque@baylibre.com>");
+MODULE_DESCRIPTION("Amlogic thermal driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


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

^ permalink raw reply related

* Re: [PATCH v8 04/14] media: rkisp1: add Rockchip MIPI Synopsys DPHY driver
From: Helen Koike @ 2019-08-21 21:46 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: devicetree, eddie.cai.linux, kernel, heiko, jacob2.chen,
	jeffy.chen, zyc, linux-kernel, tfiga, linux-rockchip,
	Sakari Ailus, sakari.ailus, zhengsq, hans.verkuil, mchehab,
	ezequiel, linux-arm-kernel, linux-media
In-Reply-To: <20190815175440.GW5011@pendragon.ideasonboard.com>

Hi Laurent,

Thanks for your review, I just have some comments/questions below.

On 8/15/19 2:54 PM, Laurent Pinchart wrote:
> Hi Helen,
> 
> Thank you for the patch.
> 
> On Wed, Aug 07, 2019 at 10:37:55AM -0300, Helen Koike wrote:
>> On 8/7/19 10:05 AM, Sakari Ailus wrote:
>>> On Tue, Jul 30, 2019 at 03:42:46PM -0300, Helen Koike wrote:
>>>> From: Jacob Chen <jacob2.chen@rock-chips.com>
>>>>
>>>> This commit adds a subdev driver for Rockchip MIPI Synopsys DPHY driver
>>>>
>>>> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
>>>> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
>>>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>>>> [migrate to phy framework]
>>>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>>>> [update for upstream]
>>>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>>>>
>>>> ---
>>>>
>>>> Changes in v8:
>>>> - Remove boiler plate license text
>>>>
>>>> Changes in v7:
>>>> - Migrate dphy specific code from
>>>> drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
>>>> to drivers/phy/rockchip/phy-rockchip-dphy.c
>>>> - Drop support for rk3288
>>>> - Drop support for dphy txrx
>>>> - code styling and checkpatch fixes
>>>>
>>>>  drivers/phy/rockchip/Kconfig             |   8 +
>>>>  drivers/phy/rockchip/Makefile            |   1 +
>>>>  drivers/phy/rockchip/phy-rockchip-dphy.c | 408 +++++++++++++++++++++++
>>>>  3 files changed, 417 insertions(+)
>>>>  create mode 100644 drivers/phy/rockchip/phy-rockchip-dphy.c
>>>>
>>>> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
>>>> index c454c90cd99e..afd072f135e6 100644
>>>> --- a/drivers/phy/rockchip/Kconfig
>>>> +++ b/drivers/phy/rockchip/Kconfig
>>>> @@ -9,6 +9,14 @@ config PHY_ROCKCHIP_DP
>>>>  	help
>>>>  	  Enable this to support the Rockchip Display Port PHY.
>>>>  
>>>> +config PHY_ROCKCHIP_DPHY
>>>> +	tristate "Rockchip MIPI Synopsys DPHY driver"
> 
> How much of this PHY is Rockchip-specific ? Would it make sense to turn
> it into a Synopsys DPHY driver, with some Rockchip glue ? I suppose this
> could always be done later, if needed (and I also suppose there's no
> existing driver in drivers/phy/ that support the same Synopsys IP).
> 
>>>> +	depends on ARCH_ROCKCHIP && OF
>>>
>>> How about (...) || COMPILE_TEST ?
>>>
>>>> +	select GENERIC_PHY_MIPI_DPHY
>>>> +	select GENERIC_PHY
>>>> +	help
>>>> +	  Enable this to support the Rockchip MIPI Synopsys DPHY.
>>>> +
>>>>  config PHY_ROCKCHIP_EMMC
>>>>  	tristate "Rockchip EMMC PHY Driver"
>>>>  	depends on ARCH_ROCKCHIP && OF
>>>> diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile
>>>> index fd21cbaf40dd..f62e9010bcaf 100644
>>>> --- a/drivers/phy/rockchip/Makefile
>>>> +++ b/drivers/phy/rockchip/Makefile
>>>> @@ -1,5 +1,6 @@
>>>>  # SPDX-License-Identifier: GPL-2.0
>>>>  obj-$(CONFIG_PHY_ROCKCHIP_DP)		+= phy-rockchip-dp.o
>>>> +obj-$(CONFIG_PHY_ROCKCHIP_DPHY)		+= phy-rockchip-dphy.o
>>>>  obj-$(CONFIG_PHY_ROCKCHIP_EMMC)		+= phy-rockchip-emmc.o
>>>>  obj-$(CONFIG_PHY_ROCKCHIP_INNO_HDMI)	+= phy-rockchip-inno-hdmi.o
>>>>  obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
>>>> diff --git a/drivers/phy/rockchip/phy-rockchip-dphy.c b/drivers/phy/rockchip/phy-rockchip-dphy.c
>>>> new file mode 100644
>>>> index 000000000000..3a29976c2dff
>>>> --- /dev/null
>>>> +++ b/drivers/phy/rockchip/phy-rockchip-dphy.c
>>>> @@ -0,0 +1,408 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>> +/*
>>>> + * Rockchip MIPI Synopsys DPHY driver
>>>> + *
>>>> + * Based on:
>>>> + *
>>>> + * Copyright (C) 2016 FuZhou Rockchip Co., Ltd.
>>>> + * Author: Yakir Yang <ykk@@rock-chips.com>
>>>> + */
>>>> +
>>>> +#include <linux/clk.h>
>>>> +#include <linux/io.h>
>>>> +#include <linux/mfd/syscon.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_device.h>
>>>> +#include <linux/phy/phy.h>
>>>> +#include <linux/phy/phy-mipi-dphy.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/regmap.h>
>>>> +
>>>> +#define RK3399_GRF_SOC_CON9	0x6224
>>>> +#define RK3399_GRF_SOC_CON21	0x6254
>>>> +#define RK3399_GRF_SOC_CON22	0x6258
>>>> +#define RK3399_GRF_SOC_CON23	0x625c
>>>> +#define RK3399_GRF_SOC_CON24	0x6260
>>>> +#define RK3399_GRF_SOC_CON25	0x6264
>>>> +#define RK3399_GRF_SOC_STATUS1	0xe2a4
>>>> +
>>>> +#define CLOCK_LANE_HS_RX_CONTROL		0x34
>>>> +#define LANE0_HS_RX_CONTROL			0x44
>>>> +#define LANE1_HS_RX_CONTROL			0x54
>>>> +#define LANE2_HS_RX_CONTROL			0x84
>>>> +#define LANE3_HS_RX_CONTROL			0x94
>>>> +#define HS_RX_DATA_LANES_THS_SETTLE_CONTROL	0x75
>>>> +
>>>> +#define MAX_DPHY_CLK 8
>>>> +
>>>> +#define PHY_TESTEN_ADDR			(0x1 << 16)
>>>> +#define PHY_TESTEN_DATA			(0x0 << 16)
>>>> +#define PHY_TESTCLK			(0x1 << 1)
>>>> +#define PHY_TESTCLR			(0x1 << 0)
> 
> Maybe s/0x// for the previous four lines ?
> 
>>>> +#define THS_SETTLE_COUNTER_THRESHOLD	0x04
>>>> +
>>>> +#define HIWORD_UPDATE(val, mask, shift) \
>>>> +	((val) << (shift) | (mask) << ((shift) + 16))
> 
> As you use this in a single place, I would inline it, possibly with a
> small comment that explains what's happening.
> 
>>>> +
>>>> +#define GRF_SOC_CON12                           0x0274
>>>> +
>>>> +#define GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK   BIT(20)
>>>> +#define GRF_EDP_REF_CLK_SEL_INTER               BIT(4)
>>>> +
>>>> +#define GRF_EDP_PHY_SIDDQ_HIWORD_MASK           BIT(21)
>>>> +#define GRF_EDP_PHY_SIDDQ_ON                    0
>>>> +#define GRF_EDP_PHY_SIDDQ_OFF                   BIT(5)
> 
> I would recommend aligning the value of of all macros in the same way.
> 
>>>> +
>>>> +struct hsfreq_range {
>>>> +	u32 range_h;
> 
> The structure would be more compact if you turned this into a u16.
> 
>>>> +	u8 cfg_bit;
>>>> +};
>>>> +
>>>> +static const struct hsfreq_range rk3399_mipidphy_hsfreq_ranges[] = {
>>>> +	{  89, 0x00}, {  99, 0x10}, { 109, 0x20}, { 129, 0x01},
>>>> +	{ 139, 0x11}, { 149, 0x21}, { 169, 0x02}, { 179, 0x12},
>>>> +	{ 199, 0x22}, { 219, 0x03}, { 239, 0x13}, { 249, 0x23},
>>>> +	{ 269, 0x04}, { 299, 0x14}, { 329, 0x05}, { 359, 0x15},
>>>> +	{ 399, 0x25}, { 449, 0x06}, { 499, 0x16}, { 549, 0x07},
>>>> +	{ 599, 0x17}, { 649, 0x08}, { 699, 0x18}, { 749, 0x09},
>>>> +	{ 799, 0x19}, { 849, 0x29}, { 899, 0x39}, { 949, 0x0a},
>>>> +	{ 999, 0x1a}, {1049, 0x2a}, {1099, 0x3a}, {1149, 0x0b},
>>>> +	{1199, 0x1b}, {1249, 0x2b}, {1299, 0x3b}, {1349, 0x0c},
>>>> +	{1399, 0x1c}, {1449, 0x2c}, {1500, 0x3c}
> 
> Maybe s/{/{ / and s/}/ }/ to give it a bit more air ? :-)
> 
>>>> +};
>>>> +
>>>> +static const char * const rk3399_mipidphy_clks[] = {
>>>> +	"dphy-ref",
>>>> +	"dphy-cfg",
>>>> +	"grf",
>>>> +};
>>>> +
>>>> +enum dphy_reg_id {
>>>> +	GRF_DPHY_RX0_TURNDISABLE = 0,
>>>> +	GRF_DPHY_RX0_FORCERXMODE,
>>>> +	GRF_DPHY_RX0_FORCETXSTOPMODE,
>>>> +	GRF_DPHY_RX0_ENABLE,
>>>> +	GRF_DPHY_RX0_TESTCLR,
>>>> +	GRF_DPHY_RX0_TESTCLK,
>>>> +	GRF_DPHY_RX0_TESTEN,
>>>> +	GRF_DPHY_RX0_TESTDIN,
>>>> +	GRF_DPHY_RX0_TURNREQUEST,
>>>> +	GRF_DPHY_RX0_TESTDOUT,
>>>> +	GRF_DPHY_TX0_TURNDISABLE,
>>>> +	GRF_DPHY_TX0_FORCERXMODE,
>>>> +	GRF_DPHY_TX0_FORCETXSTOPMODE,
>>>> +	GRF_DPHY_TX0_TURNREQUEST,
>>>> +	GRF_DPHY_TX1RX1_TURNDISABLE,
>>>> +	GRF_DPHY_TX1RX1_FORCERXMODE,
>>>> +	GRF_DPHY_TX1RX1_FORCETXSTOPMODE,
>>>> +	GRF_DPHY_TX1RX1_ENABLE,
>>>> +	GRF_DPHY_TX1RX1_MASTERSLAVEZ,
>>>> +	GRF_DPHY_TX1RX1_BASEDIR,
>>>> +	GRF_DPHY_TX1RX1_ENABLECLK,
>>>> +	GRF_DPHY_TX1RX1_TURNREQUEST,
>>>> +	GRF_DPHY_RX1_SRC_SEL,
>>>> +	/* rk3288 only */
>>>> +	GRF_CON_DISABLE_ISP,
>>>> +	GRF_CON_ISP_DPHY_SEL,
>>>> +	GRF_DSI_CSI_TESTBUS_SEL,
>>>> +	GRF_DVP_V18SEL,
>>>> +	/* below is for rk3399 only */
>>>> +	GRF_DPHY_RX0_CLK_INV_SEL,
>>>> +	GRF_DPHY_RX1_CLK_INV_SEL,
>>>> +};
>>>> +
>>>> +struct dphy_reg {
>>>> +	u32 offset;
>>>> +	u32 mask;
>>>> +	u32 shift;
> 
> The offset should hold in 16 bits and the mask and shift in 8 bits. That
> would save space in the table below.
> 
>>>> +};
>>>> +
>>>> +#define PHY_REG(_offset, _width, _shift) \
>>>> +	{ .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, }
>>>> +
>>>> +static const struct dphy_reg rk3399_grf_dphy_regs[] = {
>>>> +	[GRF_DPHY_RX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON9, 4, 0),
>>>> +	[GRF_DPHY_RX0_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 10),
>>>> +	[GRF_DPHY_RX1_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 11),
>>>> +	[GRF_DPHY_RX0_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 0),
>>>> +	[GRF_DPHY_RX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 4),
>>>> +	[GRF_DPHY_RX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 8),
>>>> +	[GRF_DPHY_RX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 12),
>>>> +	[GRF_DPHY_TX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 0),
>>>> +	[GRF_DPHY_TX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 4),
>>>> +	[GRF_DPHY_TX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 8),
>>>> +	[GRF_DPHY_TX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 12),
>>>> +	[GRF_DPHY_TX1RX1_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 0),
>>>> +	[GRF_DPHY_TX1RX1_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 4),
>>>> +	[GRF_DPHY_TX1RX1_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 8),
>>>> +	[GRF_DPHY_TX1RX1_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 12),
>>>> +	[GRF_DPHY_TX1RX1_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON24, 4, 0),
>>>> +	[GRF_DPHY_RX1_SRC_SEL] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 4),
>>>> +	[GRF_DPHY_TX1RX1_BASEDIR] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 5),
>>>> +	[GRF_DPHY_TX1RX1_ENABLECLK] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 6),
>>>> +	[GRF_DPHY_TX1RX1_MASTERSLAVEZ] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 7),
>>>> +	[GRF_DPHY_RX0_TESTDIN] = PHY_REG(RK3399_GRF_SOC_CON25, 8, 0),
>>>> +	[GRF_DPHY_RX0_TESTEN] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 8),
>>>> +	[GRF_DPHY_RX0_TESTCLK] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 9),
>>>> +	[GRF_DPHY_RX0_TESTCLR] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 10),
>>>> +	[GRF_DPHY_RX0_TESTDOUT] = PHY_REG(RK3399_GRF_SOC_STATUS1, 8, 0),
> 
> The annoying part with such an indirection is that you can't really
> write multiple fields in a single register with a single operation.
> Is
> the register mapping completely different between the rk3288 and the
> rk3399, or are the fields grouped in registers in a similar way ? In the
> latter case we could possibly optimise it.

This would be the rk3288 version:

+static const struct dphy_reg rk3288_grf_dphy_regs[] = {
+	[GRF_CON_DISABLE_ISP] = PHY_REG(RK3288_GRF_SOC_CON6, 1, 0),
+	[GRF_CON_ISP_DPHY_SEL] = PHY_REG(RK3288_GRF_SOC_CON6, 1, 1),
+	[GRF_DSI_CSI_TESTBUS_SEL] = PHY_REG(RK3288_GRF_SOC_CON6, 1, 14),
+	[GRF_DPHY_TX0_TURNDISABLE] = PHY_REG(RK3288_GRF_SOC_CON8, 4, 0),
+	[GRF_DPHY_TX0_FORCERXMODE] = PHY_REG(RK3288_GRF_SOC_CON8, 4, 4),
+	[GRF_DPHY_TX0_FORCETXSTOPMODE] = PHY_REG(RK3288_GRF_SOC_CON8, 4, 8),
+	[GRF_DPHY_TX1RX1_TURNDISABLE] = PHY_REG(RK3288_GRF_SOC_CON9, 4, 0),
+	[GRF_DPHY_TX1RX1_FORCERXMODE] = PHY_REG(RK3288_GRF_SOC_CON9, 4, 4),
+	[GRF_DPHY_TX1RX1_FORCETXSTOPMODE] = PHY_REG(RK3288_GRF_SOC_CON9, 4, 8),
+	[GRF_DPHY_TX1RX1_ENABLE] = PHY_REG(RK3288_GRF_SOC_CON9, 4, 12),
+	[GRF_DPHY_RX0_TURNDISABLE] = PHY_REG(RK3288_GRF_SOC_CON10, 4, 0),
+	[GRF_DPHY_RX0_FORCERXMODE] = PHY_REG(RK3288_GRF_SOC_CON10, 4, 4),
+	[GRF_DPHY_RX0_FORCETXSTOPMODE] = PHY_REG(RK3288_GRF_SOC_CON10, 4, 8),
+	[GRF_DPHY_RX0_ENABLE] = PHY_REG(RK3288_GRF_SOC_CON10, 4, 12),
+	[GRF_DPHY_RX0_TESTCLR] = PHY_REG(RK3288_GRF_SOC_CON14, 1, 0),
+	[GRF_DPHY_RX0_TESTCLK] = PHY_REG(RK3288_GRF_SOC_CON14, 1, 1),
+	[GRF_DPHY_RX0_TESTEN] = PHY_REG(RK3288_GRF_SOC_CON14, 1, 2),
+	[GRF_DPHY_RX0_TESTDIN] = PHY_REG(RK3288_GRF_SOC_CON14, 8, 3),
+	[GRF_DPHY_TX1RX1_ENABLECLK] = PHY_REG(RK3288_GRF_SOC_CON14, 1, 12),
+	[GRF_DPHY_RX1_SRC_SEL] = PHY_REG(RK3288_GRF_SOC_CON14, 1, 13),
+	[GRF_DPHY_TX1RX1_MASTERSLAVEZ] = PHY_REG(RK3288_GRF_SOC_CON14, 1, 14),
+	[GRF_DPHY_TX1RX1_BASEDIR] = PHY_REG(RK3288_GRF_SOC_CON14, 1, 15),
+	[GRF_DPHY_RX0_TURNREQUEST] = PHY_REG(RK3288_GRF_SOC_CON15, 4, 0),
+	[GRF_DPHY_TX1RX1_TURNREQUEST] = PHY_REG(RK3288_GRF_SOC_CON15, 4, 4),
+	[GRF_DPHY_TX0_TURNREQUEST] = PHY_REG(RK3288_GRF_SOC_CON15, 3, 8),
+	[GRF_DVP_V18SEL] = PHY_REG(RK3288_GRF_IO_VSEL, 1, 1),
+	[GRF_DPHY_RX0_TESTDOUT] = PHY_REG(RK3288_GRF_SOC_STATUS21, 8, 0),
+};

Which seems different mask and shifts from rk3399. If you have any ideas in
how to optimize this I would appreciate it.

> 
>>>> +};
>>>> +
>>>> +struct dphy_drv_data {
>>>> +	const char * const *clks;
>>>> +	int num_clks;
> 
> This is never negative, you can make it an unsigned int.
> 
>>>> +	const struct hsfreq_range *hsfreq_ranges;
>>>> +	int num_hsfreq_ranges;
> 
> Same here.
> 
>>>> +	const struct dphy_reg *regs;
>>>> +};
>>>> +
>>>> +struct rockchip_dphy {
>>>> +	struct device *dev;
>>>> +	struct regmap *grf;
>>>> +	const struct dphy_reg *grf_regs;
>>>> +	struct clk_bulk_data clks[MAX_DPHY_CLK];
>>>> +
>>>> +	const struct dphy_drv_data *drv_data;
>>>> +	struct phy_configure_opts_mipi_dphy config;
>>>> +};
>>>> +
>>>> +static inline void write_grf_reg(struct rockchip_dphy *priv,
>>>> +				 int index, u8 value)
> 
> Maybe unsigned int index ?
> 
>>>> +{
>>>> +	const struct dphy_reg *reg = &priv->grf_regs[index];
>>>> +	unsigned int val = HIWORD_UPDATE(value, reg->mask, reg->shift);
>>>> +
>>>> +	WARN_ON(!reg->offset);
>>>> +	regmap_write(priv->grf, reg->offset, val);
>>>> +}
>>>> +
>>>> +static void mipidphy0_wr_reg(struct rockchip_dphy *priv,
>>>> +			     u8 test_code, u8 test_data)
> 
> Function (and structure) names have different prefixes, would it make
> sense to standardise them ? Maybe rockchip_dphy_ ? Or rk_dphy_ for a
> shorter version ? This could become rk_dphy_write_dphy(), and the
> previous function rk_dphy_write_grf().
> 
>>>> +{
>>>> +	/*
>>>> +	 * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content
>>>> +	 * is latched internally as the current test code. Test data is
>>>> +	 * programmed internally by rising edge on TESTCLK.
>>>> +	 */
> 
> I've never understood why PHYs tend to have a register named TEST that
> contains way more than test data :-)
> 
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_code);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 1);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 0);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 0);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_data);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
>>>> +}
>>>> +
>>>> +/* should be move to power_on */
> 
> s/move/moved/
> 
> Do you mean merging the two functions together ? What prevents from
> doing so ? 

Nothing really, this is a left over command as mipidphy_rx_stream_on() is already
being called from power_on, and I don't think we should merge it because
in the future we'll probably going to have mipidphy_txrx_stream_on() for dphy1.

> 
>>>> +static int mipidphy_rx_stream_on(struct rockchip_dphy *priv)
>>>> +{
>>>> +	const struct dphy_drv_data *drv_data = priv->drv_data;
>>>> +	const struct hsfreq_range *hsfreq_ranges = drv_data->hsfreq_ranges;
>>>> +	struct phy_configure_opts_mipi_dphy *config = &priv->config;
>>>> +	unsigned int i, hsfreq = 0, data_rate_mbps = config->hs_clk_rate;
>>>> +	int num_hsfreq_ranges = drv_data->num_hsfreq_ranges;
>>>> +
>>>> +	do_div(data_rate_mbps, 1000 * 1000);
>>>> +
>>>> +	dev_dbg(priv->dev, "%s: lanes %d - data_rate_mbps %u\n",
>>>> +		__func__, config->lanes, data_rate_mbps);
>>>> +
>>>> +	for (i = 0; i < num_hsfreq_ranges; i++) {
>>>> +		if (hsfreq_ranges[i].range_h >= data_rate_mbps) {
>>>> +			hsfreq = hsfreq_ranges[i].cfg_bit;
>>>> +			break;
>>>> +		}
>>>> +	}
> 
> As num_hsfreq_ranges and hsfreq_ranges are only used in this loop, I
> would remove the local variables.
> 
>>>> +
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_FORCERXMODE, 0);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_FORCETXSTOPMODE, 0);
>>>> +
>>>> +	/* Disable lan turn around, which is ignored in receive mode */
> 
> Is it "lan turn around", or "lane turn around" ?
> 
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TURNREQUEST, 0);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TURNDISABLE, 0xf);
>>>> +
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_ENABLE, GENMASK(config->lanes - 1, 0));
>>>> +
>>>> +	/* dphy start */
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 1);
>>>> +	usleep_range(100, 150);
>>>> +	write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 0);
>>>> +	usleep_range(100, 150);
>>>> +
>>>> +	/* set clock lane */
>>>> +	/* HS hsfreq_range & lane 0  settle bypass */
>>>> +	mipidphy0_wr_reg(priv, CLOCK_LANE_HS_RX_CONTROL, 0);
>>>> +	/* HS RX Control of lane0 */
>>>> +	mipidphy0_wr_reg(priv, LANE0_HS_RX_CONTROL, hsfreq << 1);
>>>> +	/* HS RX Control of lane1 */
>>>> +	mipidphy0_wr_reg(priv, LANE1_HS_RX_CONTROL, 0);
>>>> +	/* HS RX Control of lane2 */
>>>> +	mipidphy0_wr_reg(priv, LANE2_HS_RX_CONTROL, 0);
>>>> +	/* HS RX Control of lane3 */
>>>> +	mipidphy0_wr_reg(priv, LANE3_HS_RX_CONTROL, 0);
> 
> Does this hardcode usage of a single lane ?

Rockchip seems to uses TEST* registers to set the hsfreqrange.
It mentions the test code 0x44 (which is LANE0_HS_RX_CONTROL)
but it doesn't mention the others lanes.

Replacing those call by
mipidphy0_wr_reg(priv, LANEx_HS_RX_CONTROL, hsfreq << 1);
seems to be working.

I can check if this changes the datarate (I just need to figure a proper
way to test this or get some docs).

Thanks for spotting this.

> 
>>>> +	/* HS RX Data Lanes Settle State Time Control */
>>>> +	mipidphy0_wr_reg(priv, HS_RX_DATA_LANES_THS_SETTLE_CONTROL,
>>>> +			 THS_SETTLE_COUNTER_THRESHOLD);
>>>> +
>>>> +	/* Normal operation */
>>>> +	mipidphy0_wr_reg(priv, 0x0, 0);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int rockchip_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>>>> +{
>>>> +	struct rockchip_dphy *priv = phy_get_drvdata(phy);
>>>> +	int ret;
>>>> +
>>>> +	/* pass with phy_mipi_dphy_get_default_config (with pixel rate?) */
> 
> I'm not sure to understand what this means.
> 

iirc, the question is if we should fail when phy_mipi_dphy_config_validate() fails,
or if we should use a default config.

Looking at other examples, is seems that only two drivers call
phy_mipi_dphy_get_default_config() in a totally diferent context, not in mipi path.
So I guess I would just remove this comment if this is ok with you.


>>>> +	ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	memcpy(&priv->config, opts, sizeof(priv->config));
>>>
>>> You could to:
>>>
>>> 	priv->config = *opts;
>>>
>>> Up to you. Some people like memcpy(). :-)
>>
>> your way is better thanks!
>>
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int rockchip_dphy_power_on(struct phy *phy)
>>>> +{
>>>> +	struct rockchip_dphy *priv = phy_get_drvdata(phy);
>>>> +	int ret;
>>>> +
>>>> +	ret = clk_bulk_enable(priv->drv_data->num_clks, priv->clks);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	return mipidphy_rx_stream_on(priv);
> 
> Should you call clk_bulk_disable() if mipidphy_rx_stream_on() fails ?
> Actually that function never fails, so I'd make it a void function, and
> return 0 here.

Ack, I made it void, I'll send it in the next version.

> 
> What happens if the clock rate is higher than the maximum supported by
> the PHY ? Shouldn't rockchip_dphy_configure() fail in that case ?

This is checked in function mipidphy_rx_stream_on(), if it is higher we just
configure the maximum supported rate. Is this ok?

> 
>>>> +}
>>>> +
>>>> +static int rockchip_dphy_power_off(struct phy *phy)
>>>> +{
>>>> +	struct rockchip_dphy *priv = phy_get_drvdata(phy);
>>>> +
> 
> No need to write any register ? That's scary, what will happen on the
> next power on, when the clocks gets enabled ?

Just for testing, I hacked the code to only call mipidphy_rx_stream_on() once,
when streaming for the first time, then I don't call it anymore and starting/stopping
streaming always works, so I guess it keeps the previous configuration when clocks
get enabled.
I wonder if this can be a problem when switching from dphy rx to txrx, but for now
we just support rx.

Maybe just calling rk_dphy_write_grf(priv, GRF_DPHY_RX0_ENABLE, 0) is enough.

> 
>>>> +	clk_bulk_disable(priv->drv_data->num_clks, priv->clks);
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int rockchip_dphy_init(struct phy *phy)
>>>> +{
>>>> +	struct rockchip_dphy *priv = phy_get_drvdata(phy);
>>>> +	int ret;
>>>> +
>>>> +	ret = clk_bulk_prepare(priv->drv_data->num_clks, priv->clks);
>>>
>>> return ...;
>>>
>>>> +	if (ret)
>>>> +		return ret;
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int rockchip_dphy_exit(struct phy *phy)
>>>> +{
>>>> +	struct rockchip_dphy *priv = phy_get_drvdata(phy);
>>>> +
>>>> +	clk_bulk_unprepare(priv->drv_data->num_clks, priv->clks);
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static const struct phy_ops rockchip_dphy_ops = {
>>>> +	.power_on	= rockchip_dphy_power_on,
>>>> +	.power_off	= rockchip_dphy_power_off,
>>>> +	.init		= rockchip_dphy_init,
>>>> +	.exit		= rockchip_dphy_exit,
>>>> +	.configure	= rockchip_dphy_configure,
>>>> +	.owner		= THIS_MODULE,
>>>> +};
>>>> +
>>>> +static const struct dphy_drv_data rk3399_mipidphy_drv_data = {
>>>> +	.clks = rk3399_mipidphy_clks,
>>>> +	.num_clks = ARRAY_SIZE(rk3399_mipidphy_clks),
>>>> +	.hsfreq_ranges = rk3399_mipidphy_hsfreq_ranges,
>>>> +	.num_hsfreq_ranges = ARRAY_SIZE(rk3399_mipidphy_hsfreq_ranges),
>>>> +	.regs = rk3399_grf_dphy_regs,
>>>
>>> Do you expect to support more of the similar PHY(s) --- are there such? If
>>> not, you could put these in the code that uses them.
>>
>> Yes, for rk3288 in the future.
>>
>>>> +};
>>>> +
>>>> +static const struct of_device_id rockchip_dphy_dt_ids[] = {
>>>> +	{
>>>> +		.compatible = "rockchip,rk3399-mipi-dphy",
>>>> +		.data = &rk3399_mipidphy_drv_data,
>>>> +	},
>>>> +	{}
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, rockchip_dphy_dt_ids);
>>>> +
>>>> +static int rockchip_dphy_probe(struct platform_device *pdev)
>>>> +{
>>>> +	struct device *dev = &pdev->dev;
>>>> +	struct device_node *np = dev->of_node;
>>>> +	const struct dphy_drv_data *drv_data;
>>>> +	struct phy_provider *phy_provider;
>>>> +	const struct of_device_id *of_id;
>>>> +	struct rockchip_dphy *priv;
>>>> +	struct regmap *grf;
>>>> +	struct phy *phy;
>>>> +	unsigned int i;
>>>> +	int ret;
>>>> +
>>>> +	if (!dev->parent || !dev->parent->of_node)
>>>> +		return -ENODEV;
>>>> +
>>>> +	if (platform_get_resource(pdev, IORESOURCE_MEM, 0)) {
>>>> +		dev_err(&pdev->dev, "Rockchip DPHY driver only suports rx\n");
> 
> You can replace pdev->dev with dev here and below.
> 
> s/rx/RX mode/ ?
> 
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>>> +	if (!priv)
>>>> +		return -ENOMEM;
>>>> +	priv->dev = dev;
>>>> +
>>>> +	grf = syscon_node_to_regmap(dev->parent->of_node);
>>>> +	if (IS_ERR(grf)) {
>>>> +		grf = syscon_regmap_lookup_by_phandle(dev->of_node,
>>>> +						      "rockchip,grf");
>>>> +		if (IS_ERR(grf)) {
>>>> +			dev_err(dev, "Can't find GRF syscon\n");
>>>> +			return -ENODEV;
>>>> +		}
>>>> +	}
>>>> +	priv->grf = grf;
>>>> +
>>>> +	of_id = of_match_device(rockchip_dphy_dt_ids, dev);
>>>> +	if (!of_id)
>>>> +		return -EINVAL;
>>>> +
>>>> +	drv_data = of_id->data;
>>>> +	priv->grf_regs = drv_data->regs;
> 
> Do you have to store grf_regs in priv, or could it be accessed through
> priv->drv_data->regs ?
> 
>>>> +	priv->drv_data = drv_data;
>>>> +	for (i = 0; i < drv_data->num_clks; i++)
>>>> +		priv->clks[i].id = drv_data->clks[i];
>>>> +	ret = devm_clk_bulk_get(&pdev->dev, drv_data->num_clks, priv->clks);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	phy = devm_phy_create(dev, np, &rockchip_dphy_ops);
>>>> +	if (IS_ERR(phy)) {
>>>> +		dev_err(dev, "failed to create phy\n");
>>>> +		return PTR_ERR(phy);
>>>> +	}
>>>> +	phy_set_drvdata(phy, priv);
>>>> +
>>>> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>>>> +
>>>> +	return PTR_ERR_OR_ZERO(phy_provider);
>>>> +}
>>>> +
>>>> +static struct platform_driver rockchip_dphy_driver = {
>>>> +	.probe = rockchip_dphy_probe,
>>>> +	.driver = {
>>>> +		.name	= "rockchip-mipi-dphy",
>>>> +		.of_match_table = rockchip_dphy_dt_ids,
>>>> +	},
>>>> +};
>>>> +module_platform_driver(rockchip_dphy_driver);
>>>> +
>>>> +MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>");
>>>> +MODULE_DESCRIPTION("Rockchip MIPI Synopsys DPHY driver");
>>>> +MODULE_LICENSE("Dual MIT/GPL");
> 
> Overall this is quite good, there are only small issues.
> 

Thank you a lot for your review
Helen

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

^ permalink raw reply

* Re: [PATCH v3 0/6] Add support of New Amlogic temperature sensor for G12 SoCs
From: Kevin Hilman @ 2019-08-21 21:30 UTC (permalink / raw)
  To: Guillaume La Roque, daniel.lezcano
  Cc: devicetree, linux-amlogic, linux-kernel, linux-arm-kernel,
	linux-pm
In-Reply-To: <7hd0hd3mme.fsf@baylibre.com>

Kevin Hilman <khilman@baylibre.com> writes:

> Guillaume La Roque <glaroque@baylibre.com> writes:
>
>> This patchs series add support of New Amlogic temperature sensor and minimal
>> thermal zone for SEI510 and ODROID-N2 boards.
>>
>> First implementation was doing on IIO[1] but after comments i move on thermal framework.
>> Formulas and calibration values come from amlogic.
>>
>> Changes since v2:
>>   - fix yaml documention 
>>   - remove unneeded status variable for temperature-sensor node
>>   - rework driver after Martin review
>>   - add some information in commit message
>>
>> Changes since v1:
>>   - fix enum vs const in documentation
>>   - fix error with thermal-sensor-cells value set to 1 instead of 0
>>   - add some dependencies needed to add cooling-maps
>>
>> Dependencies :
>> - patch 3,4 & 5: depends on Neil's patch and series :
>>               - missing dwc2 phy-names[2]
>>               - patchsets to add DVFS on G12a[3] which have deps on [4] and [5]
>>
>> [1] https://lore.kernel.org/linux-amlogic/20190604144714.2009-1-glaroque@baylibre.com/
>> [2] https://lore.kernel.org/linux-amlogic/20190625123647.26117-1-narmstrong@baylibre.com/
>> [3] https://lore.kernel.org/linux-amlogic/20190729132622.7566-1-narmstrong@baylibre.com/
>> [4] https://lore.kernel.org/linux-amlogic/20190731084019.8451-5-narmstrong@baylibre.com/
>> [5] https://lore.kernel.org/linux-amlogic/20190729132622.7566-3-narmstrong@baylibre.com/
>
> Thank you for the detailed list of dependencies!  Much appreciated.
>
> With all the deps, I tested this on sei510 and odroid-n2, and basic
> functionality seems to work.
>
> As discussed off-list: it would be nice to have an example of how
> cpufreq could be used as a cooling device for hot temperatures.  The
> vendor kernel has some trip points that could be included as examples,
> or even included as extra patches.
>
> Also the driver patch is missing the two main thermal maintainers, so
> please resend at least the driver and bindings including them.

Forgot to add...

Tested-by: Kevin Hilman <khilman@baylibre.com>

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

^ permalink raw reply

* Re: [PATCH v3 0/6] Add support of New Amlogic temperature sensor for G12 SoCs
From: guillaume La Roque @ 2019-08-21 21:19 UTC (permalink / raw)
  To: Christian Hewitt
  Cc: devicetree, linux-pm, khilman, daniel.lezcano, linux-kernel,
	linux-amlogic, linux-arm-kernel
In-Reply-To: <357FACA6-6EAC-4E9D-B6F7-4D8D0D59A20A@gmail.com>

Hi Christian,


thanks for testing on this 3 boards.


guillaume

On 8/14/19 2:24 PM, Christian Hewitt wrote:
> On 6 Aug 2019, at 5:05 pm, Guillaume La Roque <glaroque@baylibre.com> wrote:
>> This patchs series add support of New Amlogic temperature sensor and minimal
>> thermal zone for SEI510 and ODROID-N2 boards.
>>
>> First implementation was doing on IIO[1] but after comments i move on thermal framework.
>> Formulas and calibration values come from amlogic.
>>
>> Changes since v2:
>>  - fix yaml documention 
>>  - remove unneeded status variable for temperature-sensor node
>>  - rework driver after Martin review
>>  - add some information in commit message
>>
>> Changes since v1:
>>  - fix enum vs const in documentation
>>  - fix error with thermal-sensor-cells value set to 1 instead of 0
>>  - add some dependencies needed to add cooling-maps
>>
>> Dependencies :
>> - patch 3,4 & 5: depends on Neil's patch and series :
>>              - missing dwc2 phy-names[2]
>>              - patchsets to add DVFS on G12a[3] which have deps on [4] and [5]
>>
>> [1] https://lore.kernel.org/linux-amlogic/20190604144714.2009-1-glaroque@baylibre.com/
>> [2] https://lore.kernel.org/linux-amlogic/20190625123647.26117-1-narmstrong@baylibre.com/
>> [3] https://lore.kernel.org/linux-amlogic/20190729132622.7566-1-narmstrong@baylibre.com/
>> [4] https://lore.kernel.org/linux-amlogic/20190731084019.8451-5-narmstrong@baylibre.com/
>> [5] https://lore.kernel.org/linux-amlogic/20190729132622.7566-3-narmstrong@baylibre.com/
>>
>> Guillaume La Roque (6):
>>  dt-bindings: thermal: Add DT bindings documentation for Amlogic
>>    Thermal
>>  thermal: amlogic: Add thermal driver to support G12 SoCs
>>  arm64: dts: amlogic: g12: add temperature sensor
>>  arm64: dts: meson: sei510: Add minimal thermal zone
>>  arm64: dts: amlogic: odroid-n2: add minimal thermal zone
>>  MAINTAINERS: add entry for Amlogic Thermal driver
> Tested-by: Christian Hewitt <christianshewitt@gmail.com>
>
> I’ve tested this series with Odroid N2 and Khadas VIM3, X96-Max. Patches to add
> support for VIM3/X96-max will be submitted once the driver is merged.
>
> VIM3:~ # dmesg | grep thermal
> [    0.046375] thermal_sys: Registered thermal governor 'step_wise'
>
> VIM3:~ # cat /sys/devices/virtual/thermal/thermal_zone0/temp
> 51300
>
> VIM3:~ # cat /sys/devices/virtual/thermal/thermal_zone1/temp
> 52800
>
> Christian

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

^ permalink raw reply

* Re: [PATCH v2 4/6] dt-bindings: serial: Document Freescale LINFlex UART
From: Rob Herring @ 2019-08-21 21:18 UTC (permalink / raw)
  To: Stefan-gabriel Mirea
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, will@kernel.org,
	corbet@lwn.net, gregkh@linuxfoundation.org, jslaby@suse.com,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, Leo Li,
	Cosmin Stefan Stoica, linux-serial@vger.kernel.org,
	catalin.marinas@arm.com, shawnguo@kernel.org,
	linux-arm-kernel@lists.infradead.org, Larisa Ileana Grigore
In-Reply-To: <20190809112853.15846-5-stefan-gabriel.mirea@nxp.com>

On Fri, Aug 09, 2019 at 11:29:14AM +0000, Stefan-gabriel Mirea wrote:
> From: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
> 
> Add documentation for the serial communication interface module (LINFlex),
> found in two instances on S32V234.
> 
> Signed-off-by: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
> Signed-off-by: Larisa Grigore <Larisa.Grigore@nxp.com>
> Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
> ---
>  .../bindings/serial/fsl,s32-linflexuart.txt   | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> new file mode 100644
> index 000000000000..957ffeaca9f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
> @@ -0,0 +1,24 @@
> +* Freescale Linflex UART

Be consistent with the name: LINFlexD?

> +
> +The LINFlexD controller implements several LIN protocol versions, as well as
> +support for full-duplex UART communication through 8-bit and 9-bit frames. The
> +Linflex UART driver enables operation only in UART mode.

What the driver supports or not is independent of the binding.

> +
> +See chapter 47 ("LINFlexD") in the reference manual[1].
> +
> +Required properties:
> +- compatible :
> +  - "fsl,s32-linflexuart" for linflex configured in uart mode which

LINFlexD?

> +  is compatible with the one integrated on S32V234 SoC

Compatibles should be SoC specific. Is 's32' specific enough to account 
for any differences or future bugs found?

> +- reg : Address and length of the register set for the device
> +- interrupts : Should contain uart interrupt
> +
> +Example:
> +uart0:serial@40053000 {

space    ^

> +	compatible = "fsl,s32-linflexuart";
> +	reg = <0x0 0x40053000 0x0 0x1000>;
> +	interrupts = <0 59 4>;
> +	status = "disabled";

Don't show status in examples.

> +};
> +
> +[1] https://www.nxp.com/webapp/Download?colCode=S32V234RM
> -- 
> 2.22.0
> 

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

^ permalink raw reply

* Re: [PATCH v2 08/14] arm64: dts: meson-gxl: fix internal phy compatible
From: Martin Blumenstingl @ 2019-08-21 21:16 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: khilman, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <20190821142043.14649-9-narmstrong@baylibre.com>

On Wed, Aug 21, 2019 at 4:23 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This fixes the following DT schemas check errors:
> meson-gxl-s805x-libretech-ac.dt.yaml: ethernet-phy@8: compatible: ['ethernet-phy-id0181.4400', 'ethernet-phy-ieee802.3-c22'] is not valid under any of the given schemas
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
based on the explanation in v1 we can program any arbitrary PHY ID so
Jerome's argument to list the PHY ID here applies
with that:
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

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

^ permalink raw reply

* Re: [PATCH v2 12/14] arm64: dts: meson-gxbb-nanopi-k2: add missing model
From: Martin Blumenstingl @ 2019-08-21 21:15 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: khilman, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <20190821142043.14649-13-narmstrong@baylibre.com>

On Wed, Aug 21, 2019 at 4:24 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This fixes the following DT schemas check errors:
> meson-gxbb-nanopi-k2.dt.yaml: /: 'model' is a required property
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

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

^ permalink raw reply

* Re: [PATCH 11/14] arm64: dts: meson-g12a-x96-max: fix compatible
From: Martin Blumenstingl @ 2019-08-21 21:14 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: khilman, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <c2d78c7c-d9a8-e486-d3b1-c1447e24284b@baylibre.com>

On Wed, Aug 21, 2019 at 4:08 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 20/08/2019 22:32, Martin Blumenstingl wrote:
> > On Wed, Aug 14, 2019 at 4:33 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >>
> >> This fixes the following DT schemas check errors:
> >> meson-g12a-x96-max.dt.yaml: /: compatible: ['amediatech,x96-max', 'amlogic,u200', 'amlogic,g12a'] is not valid under any of the given schemas
> >>
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> >
> > [...]
> >> -       compatible = "amediatech,x96-max", "amlogic,u200", "amlogic,g12a";
> >> +       compatible = "amediatech,x96-max", "amlogic,g12a";
> > only partially related: I wonder if we should add a s905x2 compatible
> > string here and to the .dts filename (just like we separate the GXL
> > variants s905x, s905d, s905w, ...)
> >
>
> We could, but AFAIK no variants of G12A are planned yet...
we already support two variants: S905X2 and S905D2 (I'm assuming that
these are similar to S905X and S905D meaning both are almost
identical)
but I guess we can stay with what we have until there's a reason to
have separate compatible strings (for example if we discover that
there is a difference that matters - like the OPP table on S922X vs
A311D)

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

^ permalink raw reply

* Re: [PATCH 04/19] dt-bindings: phy-mmp3-usb: Add bindings
From: Rob Herring @ 2019-08-21 21:13 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Mark Rutland, devicetree, Jason Cooper, Stephen Boyd,
	Marc Zyngier, Michael Turquette, Russell King,
	Kishon Vijay Abraham I, linux-arm-kernel, Olof Johansson,
	Thomas Gleixner, linux-clk, linux-kernel
In-Reply-To: <20190809093158.7969-5-lkundrak@v3.sk>

On Fri, Aug 09, 2019 at 11:31:43AM +0200, Lubomir Rintel wrote:
> This is the PHY chip for USB OTG on MMP3 platform.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  .../devicetree/bindings/phy/phy-mmp3-usb.txt     | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-mmp3-usb.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-mmp3-usb.txt b/Documentation/devicetree/bindings/phy/phy-mmp3-usb.txt
> new file mode 100644
> index 0000000000000..b9623b98151bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-mmp3-usb.txt
> @@ -0,0 +1,16 @@
> +Marvell MMP3 USB PHY
> +--------------------
> +
> +Required properties:
> +- compatible: must be "marvell,mmp3-usb-phy"
> +- #phy-cells: must be 0
> +
> +Example:
> +	usb-phy: usbphy@d4207000 {

usb-phy@...

> +		compatible = "marvell,mmp3-usb-phy";
> +		reg = <0xd4207000 0x40>;
> +		#phy-cells = <0>;
> +	};
> +
> +This document explains the device tree binding. For general information
> +about PHY subsystem refer to Documentation/phy.txt

Drop this statement.


> -- 
> 2.21.0
> 

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

^ permalink raw reply

* Re: [PATCH 03/19] dt-bindings: mrvl,intc: Add a MMP3 interrupt controller
From: Rob Herring @ 2019-08-21 21:11 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Mark Rutland, devicetree, Jason Cooper, Stephen Boyd,
	Marc Zyngier, Michael Turquette, Russell King,
	Kishon Vijay Abraham I, linux-arm-kernel, Olof Johansson,
	Thomas Gleixner, linux-clk, linux-kernel
In-Reply-To: <20190809093158.7969-4-lkundrak@v3.sk>

On Fri, Aug 09, 2019 at 11:31:42AM +0200, Lubomir Rintel wrote:
> Similar to MMP2 one, but has an extra range for the other core. The
> muxes stay the same.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  .../interrupt-controller/mrvl,intc.txt        | 23 ++++++++++++++-----
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.txt b/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.txt
> index 608fee15a4cfc..41c131d026f94 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.txt
> @@ -1,13 +1,15 @@
>  * Marvell MMP Interrupt controller
>  
>  Required properties:
> -- compatible : Should be "mrvl,mmp-intc", "mrvl,mmp2-intc" or
> -  "mrvl,mmp2-mux-intc"
> +- compatible : Should be "mrvl,mmp-intc", "mrvl,mmp2-intc",
> +  "marvell,mmp3-intc", "mrvl,mmp2-mux-intc"

Reformat to 1 valid combination per line.

>  - reg : Address and length of the register set of the interrupt controller.
>    If the interrupt controller is intc, address and length means the range
> -  of the whole interrupt controller. If the interrupt controller is mux-intc,
> -  address and length means one register. Since address of mux-intc is in the
> -  range of intc. mux-intc is secondary interrupt controller.
> +  of the whole interrupt controller. The "marvell,mmp3-intc" controller
> +  also has a secondary range for the second CPU core.  If the interrupt
> +  controller is mux-intc, address and length means one register. Since
> +  address of mux-intc is in the range of intc. mux-intc is secondary
> +  interrupt controller.
>  - reg-names : Name of the register set of the interrupt controller. It's
>    only required in mux-intc interrupt controller.
>  - interrupts : Should be the port interrupt shared by mux interrupts. It's
> @@ -20,7 +22,7 @@ Required properties:
>  - mrvl,clr-mfp-irq : Specifies the interrupt that needs to clear MFP edge
>    detection first.
>  
> -Example:
> +Examples:
>  	intc: interrupt-controller@d4282000 {
>  		compatible = "mrvl,mmp2-intc";
>  		interrupt-controller;
> @@ -29,6 +31,15 @@ Example:
>  		mrvl,intc-nr-irqs = <64>;
>  	};
>  
> +	intc: interrupt-controller@d4282000 {

What's special about this to warrant another example. Examples aren't 
supposed to be an enumeration of all possible dts entries.

> +		compatible = "marvell,mmp3-intc";
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		reg = <0xd4282000 0x1000>,
> +		      <0xd4284000 0x100>;
> +		mrvl,intc-nr-irqs = <64>;
> +	};
> +
>  	intcmux4@d4282150 {
>  		compatible = "mrvl,mmp2-mux-intc";
>  		interrupts = <4>;
> -- 
> 2.21.0
> 

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

^ permalink raw reply

* Re: [PATCH net-next v2 0/3] net: dsa: mt7530: Convert to PHYLINK and add support for port 5
From: Andrew Lunn @ 2019-08-21 21:05 UTC (permalink / raw)
  To: René van Dorst
  Cc: Florian Fainelli, Frank Wunderlich, netdev, Sean Wang, linux-mips,
	David S . Miller, linux-mediatek, John Crispin, Matthias Brugger,
	Vivien Didelot, linux-arm-kernel
In-Reply-To: <20190821144547.15113-1-opensource@vdorst.com>

On Wed, Aug 21, 2019 at 04:45:44PM +0200, René van Dorst wrote:
> 1. net: dsa: mt7530: Convert to PHYLINK API
>    This patch converts mt7530 to PHYLINK API.
> 2. dt-bindings: net: dsa: mt7530: Add support for port 5
> 3. net: dsa: mt7530: Add support for port 5
>    These 2 patches adding support for port 5 of the switch.
> 
> v1->v2:
>  * Mostly phylink improvements after review.

Hi René

You are addressing comments mostly from Russell King. It would of been
good to Cc: him on the patchset.

Andrew

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

^ permalink raw reply

* Re: [PATCH 02/19] dt-bindings: arm: mrvl: Document MMP3 compatible string
From: Rob Herring @ 2019-08-21 21:03 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Mark Rutland, devicetree, Jason Cooper, Stephen Boyd,
	Marc Zyngier, Michael Turquette, Russell King,
	Kishon Vijay Abraham I, linux-arm-kernel, Olof Johansson,
	Thomas Gleixner, linux-clk, linux-kernel
In-Reply-To: <20190809093158.7969-3-lkundrak@v3.sk>

On Fri, Aug 09, 2019 at 11:31:41AM +0200, Lubomir Rintel wrote:
> Marvel MMP3 is a successor to MMP2, containing similar peripherals with two
> PJ4B cores.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  Documentation/devicetree/bindings/arm/mrvl/mrvl.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/mrvl/mrvl.txt b/Documentation/devicetree/bindings/arm/mrvl/mrvl.txt
> index 951687528efb0..66e1e1414245b 100644
> --- a/Documentation/devicetree/bindings/arm/mrvl/mrvl.txt
> +++ b/Documentation/devicetree/bindings/arm/mrvl/mrvl.txt
> @@ -12,3 +12,7 @@ Required root node properties:
>  MMP2 Brownstone Board
>  Required root node properties:
>  	- compatible = "mrvl,mmp2-brownstone", "mrvl,mmp2";
> +
> +MMP3 SoC
> +Required root node properties:
> +	- compatible = "marvell,mmp3";

Please convert this file to DT schema before adding new SoCs.

Rob

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

^ permalink raw reply

* Re: [PATCH 5/6] dt-bindings: pinctrl: k3: Introduce pinmux definitions for J721E
From: Rob Herring @ 2019-08-21 21:02 UTC (permalink / raw)
  To: Lokesh Vutla
  Cc: Nishanth Menon, Device Tree Mailing List, Keerthy, linus.walleij,
	Tero Kristo, linux-gpio, Linux ARM Mailing List
In-Reply-To: <20190809082947.30590-6-lokeshvutla@ti.com>

On Fri, Aug 09, 2019 at 01:59:46PM +0530, Lokesh Vutla wrote:
> Add pinctrl macros for J721E SoC. These macro definitions are
> similar to that of AM6, but adding new definitions to avoid
> any naming confusions in the soc dts files.
> 
> Acked-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
>  include/dt-bindings/pinctrl/k3.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/dt-bindings/pinctrl/k3.h b/include/dt-bindings/pinctrl/k3.h
> index 45e11b6170ca..499de6216581 100644
> --- a/include/dt-bindings/pinctrl/k3.h
> +++ b/include/dt-bindings/pinctrl/k3.h
> @@ -32,4 +32,7 @@
>  #define AM65X_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
>  #define AM65X_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
>  
> +#define J721E_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
> +#define J721E_WKUP_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))

checkpatch reports a parentheses error:         (((pa) & 0x1fff) ((val) | (muxmode)))

> +
>  #endif
> -- 
> 2.22.0
> 

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

^ permalink raw reply

* [RESEND PATCH 1/1] rtc: sun6i: Allow using as wakeup source from suspend
From: Alejandro González @ 2019-08-21 21:00 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, maxime.ripard, wens, linux-rtc,
	linux-arm-kernel, linux-kernel
  Cc: linux-sunxi, Alejandro González

This patch allows userspace to set up wakeup alarms on any RTC handled by the
sun6i driver, and adds the necessary PM operations to allow resuming from
suspend when the configured wakeup alarm fires a IRQ. Of course, that the
device actually resumes depends on the suspend state and how a particular
hardware reacts to it, but that is out of scope for this patch.

I've tested these changes on a Pine H64 model B, which contains a
Allwinner H6 SoC, with the help of CONFIG_PM_TEST_SUSPEND kernel option.
These are the interesting outputs from the kernel and commands which
show that it works. As every RTC handled by this driver is largely the
same, I think that it shouldn't introduce any regression on other SoCs,
but I may be wrong.

[    1.092705] PM: test RTC wakeup from 'freeze' suspend
[    1.098230] PM: suspend entry (s2idle)
[    1.212907] PM: suspend devices took 0.080 seconds
(The SoC freezes for some seconds)
[    3.197604] PM: resume devices took 0.104 seconds
[    3.215937] PM: suspend exit

[    1.092812] PM: test RTC wakeup from 'mem' suspend
[    1.098089] PM: suspend entry (deep)
[    1.102033] PM: suspend exit
[    1.105205] PM: suspend test failed, error -22

In any case, the RTC alarm interrupt gets fired as exptected:

$ echo +5 > /sys/class/rtc/rtc0/wakealarm && sleep 5 && grep rtc /proc/interrupts
 29:          1          0          0          0     GICv2 133 Level     7000000.rtc

Signed-off-by: Alejandro González <alejandro.gonzalez.correo@gmail.com>
---
 drivers/rtc/rtc-sun6i.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index c0e75c373605..b7611e5dea3f 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -598,6 +598,33 @@ static const struct rtc_class_ops sun6i_rtc_ops = {
 	.alarm_irq_enable	= sun6i_rtc_alarm_irq_enable
 };
 
+#ifdef CONFIG_PM_SLEEP
+/* Enable IRQ wake on suspend, to wake up from RTC. */
+static int sun6i_rtc_suspend(struct device *dev)
+{
+	struct sun6i_rtc_dev *chip = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(chip->irq);
+
+	return 0;
+}
+
+/* Disable IRQ wake on resume. */
+static int sun6i_rtc_resume(struct device *dev)
+{
+	struct sun6i_rtc_dev *chip = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(chip->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(sun6i_rtc_pm_ops,
+	sun6i_rtc_suspend, sun6i_rtc_resume);
+
 static int sun6i_rtc_probe(struct platform_device *pdev)
 {
 	struct sun6i_rtc_dev *chip = sun6i_rtc;
@@ -650,6 +677,8 @@ static int sun6i_rtc_probe(struct platform_device *pdev)
 
 	clk_prepare_enable(chip->losc);
 
+	device_init_wakeup(&pdev->dev, 1);
+
 	chip->rtc = devm_rtc_device_register(&pdev->dev, "rtc-sun6i",
 					     &sun6i_rtc_ops, THIS_MODULE);
 	if (IS_ERR(chip->rtc)) {
@@ -684,6 +713,7 @@ static struct platform_driver sun6i_rtc_driver = {
 	.driver		= {
 		.name		= "sun6i-rtc",
 		.of_match_table = sun6i_rtc_dt_ids,
+		.pm = &sun6i_rtc_pm_ops,
 	},
 };
 builtin_platform_driver(sun6i_rtc_driver);
-- 
2.20.1


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

^ permalink raw reply related

* Re: [PATCH v1 1/3] gpio: mpc8xxx: add ls1088a platform gpio node DT binding description
From: Rob Herring @ 2019-08-21 20:53 UTC (permalink / raw)
  To: Hui Song
  Cc: Mark Rutland, devicetree, Song Hui, linux-gpio, Linus Walleij,
	linux-kernel, Li Yang, Bartosz Golaszewski, Shawn Guo,
	linux-arm-kernel
In-Reply-To: <20190808101628.36782-1-hui.song_1@nxp.com>

On Thu,  8 Aug 2019 18:16:26 +0800, Hui Song wrote:
> From: Song Hui <hui.song_1@nxp.com>
> 
> ls1088a and ls1028a platform share common gpio node.
> 
> Signed-off-by: Song Hui <hui.song_1@nxp.com>
> ---
>  Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

^ permalink raw reply

* Re: [PATCH net-next] net: stmmac: dwc-qos: use devm_platform_ioremap_resource() to simplify code
From: David Miller @ 2019-08-21 20:53 UTC (permalink / raw)
  To: yuehaibing
  Cc: alexandre.torgue, khilman, linux-kernel, joabreu, mcoquelin.stm32,
	netdev, peppe.cavallaro, linux-amlogic, linux-stm32,
	linux-arm-kernel
In-Reply-To: <20190821135701.46780-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Wed, 21 Aug 2019 21:57:01 +0800

> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

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

^ permalink raw reply

* Re: [PATCH net-next] net: stmmac: dwmac-anarion: use devm_platform_ioremap_resource() to simplify code
From: David Miller @ 2019-08-21 20:53 UTC (permalink / raw)
  To: yuehaibing
  Cc: alexandre.torgue, khilman, linux-kernel, joabreu, mcoquelin.stm32,
	netdev, peppe.cavallaro, linux-amlogic, linux-stm32,
	linux-arm-kernel
In-Reply-To: <20190821135550.55200-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Wed, 21 Aug 2019 21:55:50 +0800

> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

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

^ permalink raw reply

* Re: [PATCH net-next] net: stmmac: dwmac-meson: use devm_platform_ioremap_resource() to simplify code
From: David Miller @ 2019-08-21 20:53 UTC (permalink / raw)
  To: yuehaibing
  Cc: alexandre.torgue, khilman, linux-kernel, joabreu, mcoquelin.stm32,
	netdev, peppe.cavallaro, linux-amlogic, linux-stm32,
	linux-arm-kernel
In-Reply-To: <20190821135406.26200-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Wed, 21 Aug 2019 21:54:06 +0800

> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

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

^ permalink raw reply

* Re: [PATCH net-next] net: stmmac: dwmac-meson8b: use devm_platform_ioremap_resource() to simplify code
From: David Miller @ 2019-08-21 20:53 UTC (permalink / raw)
  To: yuehaibing
  Cc: alexandre.torgue, khilman, linux-kernel, joabreu, mcoquelin.stm32,
	netdev, peppe.cavallaro, linux-amlogic, linux-stm32,
	linux-arm-kernel
In-Reply-To: <20190821135130.68636-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Wed, 21 Aug 2019 21:51:30 +0800

> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

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

^ permalink raw reply

* Re: [PATCH v4] arm64: dts: imx8mq: Init rates and parents configs for clocks
From: Leonard Crestez @ 2019-08-21 20:39 UTC (permalink / raw)
  To: Daniel Baluta, shawnguo@kernel.org, Abel Vesa
  Cc: devicetree@vger.kernel.org, baruch@tkos.co.il, Jacky Bai,
	Anson Huang, ccaione@baylibre.com, andrew.smirnov@gmail.com,
	s.hauer@pengutronix.de, angus@akkea.ca, Stephen Boyd,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
	agx@sigxcpu.org, dl-linux-imx, Peng Fan, festevam@gmail.com,
	S.j. Wang, linux-arm-kernel@lists.infradead.org,
	l.stach@pengutronix.de
In-Reply-To: <20190728152040.15323-1-daniel.baluta@nxp.com>

On 28.07.2019 18:20, Daniel Baluta wrote:
> From: Abel Vesa <abel.vesa@nxp.com>
> 
> Add the initial configuration for clocks that need default parent and rate
> setting. This is based on the vendor tree clock provider parents and rates
> configuration except this is doing the setup in dts rather then using clock
> consumer API in a clock provider driver.
> 
> Note that by adding the initial rate setting for audio_pll1/audio_pll
> setting we need to remove it from imx8mq-librem5-devkit.dts

Setting default rates for audio_pll1 and audio_pll2 in soc dtsi makes a 
lot of sense to me; the intention is for one to run at a multiple of 
44.1k and another at a multiple of 48k.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index 02fbd0625318..a55d72ba2e05 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -494,6 +494,25 @@
>   				clock-names = "ckil", "osc_25m", "osc_27m",
>   				              "clk_ext1", "clk_ext2",
>   				              "clk_ext3", "clk_ext4";
> +				assigned-clocks = <&clk IMX8MQ_VIDEO_PLL1>,
> +					<&clk IMX8MQ_AUDIO_PLL1>,
> +					<&clk IMX8MQ_AUDIO_PLL2>,
> +					<&clk IMX8MQ_CLK_AHB>,
> +					<&clk IMX8MQ_CLK_NAND_USDHC_BUS>,
> +					<&clk IMX8MQ_CLK_AUDIO_AHB>,
> +					<&clk IMX8MQ_VIDEO_PLL1_REF_SEL>,
> +					<&clk IMX8MQ_CLK_NOC>;
> +				assigned-clock-parents = <0>,
> +						<0>,
> +						<0> > +						<&clk IMX8MQ_SYS1_PLL_133M>,
> +						<&clk IMX8MQ_SYS1_PLL_266M>,
> +						<&clk IMX8MQ_SYS2_PLL_500M>,
> +						<&clk IMX8MQ_CLK_27M>,
> +						<&clk IMX8MQ_SYS1_PLL_800M>;
> +				assigned-clock-rates = <593999999>,
> +						<786432000>,
> +						<722534400>;

The audio PLLs should run below 650 mHz so please use 393216000 and 
361267200 instead of 786432000 and 722534400. For the 8mm equivalent see 
commit 053a4ffe2988 ("clk: imx: imx8mm: fix audio pll setting").

You should also move the unbypassing of AUDIO_PLL1 and AUDIO_PLL2 here 
just add two more assigned-clocks and assigned-clock-parents.

--
Regards,
Leonard

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

^ permalink raw reply

* Re: [RESEND PATCH v5 4/4] dt-bindings: devfreq: exynos-bus: remove unused property
From: Rob Herring @ 2019-08-21 20:35 UTC (permalink / raw)
  To: Kamil Konieczny
  Cc: Mark Rutland, Nishanth Menon, linux-samsung-soc, linux-arm-kernel,
	Bartlomiej Zolnierkiewicz, Stephen Boyd, Viresh Kumar, linux-pm,
	linux-kernel, Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park,
	Kukjin Kim, MyungJoo Ham, devicetree, Marek Szyprowski
In-Reply-To: <20190808090234.12577-5-k.konieczny@partner.samsung.com>

On Thu, Aug 08, 2019 at 11:02:34AM +0200, Kamil Konieczny wrote:
> Remove unused DT property "exynos,voltage-tolerance".
> 
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  Documentation/devicetree/bindings/devfreq/exynos-bus.txt | 2 --
>  1 file changed, 2 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

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

^ permalink raw reply

* Re: [RFC,v4,1/4] media: dt-bindings: mt8183: Added camera ISP Pass 1
From: Rob Herring @ 2019-08-21 20:17 UTC (permalink / raw)
  To: Jungo Lin
  Cc: ryan.yu, frankie.chiu, laurent.pinchart, Rynn.Wu, suleiman,
	Jerry-ch.Chen, frederic.chen, linux-media, devicetree,
	hverkuil-cisco, shik, yuzhao, linux-mediatek, matthias.bgg,
	mchehab, linux-arm-kernel, Sean.Cheng, srv_heupstream, sj.huang,
	tfiga, zwisler, ddavenport
In-Reply-To: <20190807124803.29884-2-jungo.lin@mediatek.com>

On Wed, Aug 07, 2019 at 08:48:00PM +0800, Jungo Lin wrote:
> This patch adds DT binding document for the Pass 1 (P1) unit
> in Mediatek's camera ISP system. The Pass 1 unit grabs the sensor
> data out from the sensor interface, applies ISP image effects
> from tuning data and outputs the image data or statistics data to DRAM.
> 
> Signed-off-by: Jungo Lin <jungo.lin@mediatek.com>
> ---
>  .../bindings/media/mediatek,camisp.txt        | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,camisp.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,camisp.txt b/Documentation/devicetree/bindings/media/mediatek,camisp.txt
> new file mode 100644
> index 000000000000..fa2713acceca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,camisp.txt
> @@ -0,0 +1,73 @@
> +* Mediatek Image Signal Processor Pass 1 (ISP P1)
> +
> +The Pass 1 unit of Mediatek's camera ISP system grabs the sensor data out
> +from the sensor interface, applies ISP effects from tuning data and outputs
> +the image data and statistics data to DRAM. Furthermore, Pass 1 unit has
> +the ability to output two different resolutions frames at the same time to
> +increase the performance of the camera application.
> +
> +Required properties:
> +- compatible: Must be "mediatek,mt8183-camisp" for MT8183.
> +- reg: Physical base address of the camera function block register and
> +  length of memory mapped region. Must contain an entry for each entry
> +  in reg-names.
> +- reg-names: Must include the following entries:
> +  "cam_sys": Camera base function block
> +  "cam_uni": Camera UNI function block
> +  "cam_a": Camera ISP P1 hardware unit A
> +  "cam_b": Camera ISP P1 hardware unit B
> +  "cam_c": Camera ISP P1 hardware unit C
> +- interrupts: Must contain an entry for each entry in interrupt-names.
> +- interrupt-names : Must include the following entries:
> +  "cam_uni": Camera UNI interrupt
> +  "cam_a": Camera unit A interrupt
> +  "cam_b": Camera unit B interrupt
> +  "cam_c": Camera unit C interrupt
> +- iommus: Shall point to the respective IOMMU block with master port
> +  as argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> +  for details.
> +- clocks: A list of phandle and clock specifier pairs as listed
> +  in clock-names property, see
> +  Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
> +- clock-names: Must be "camsys_cam_cgpdn" and "camsys_camtg_cgpdn".
> +- mediatek,larb: Must contain the local arbiters in the current SoCs, see
> +  Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> +  for details.
> +- power-domains: a phandle to the power domain, see
> +  Documentation/devicetree/bindings/power/power_domain.txt for details.
> +- mediatek,scp : The node of system control processor (SCP), see
> +  Documentation/devicetree/bindings/remoteproc/mtk,scp.txt for details.
> +
> +Example:
> +SoC specific DT entry:
> +
> +		camisp: camisp@1a000000 {

Also, you can remove 2 levels of indentation here.

> +			compatible = "mediatek,mt8183-camisp", "syscon";
> +			reg = <0 0x1a000000 0 0x1000>,
> +					<0 0x1a003000 0 0x1000>,
> +					<0 0x1a004000 0 0x2000>,
> +					<0 0x1a006000 0 0x2000>,
> +					<0 0x1a008000 0 0x2000>;
> +			reg-names = "cam_sys",
> +					"cam_uni",
> +					"cam_a",
> +					"cam_b",
> +					"cam_c";
> +			interrupts = <GIC_SPI 253 IRQ_TYPE_LEVEL_LOW>,
> +					<GIC_SPI 254 IRQ_TYPE_LEVEL_LOW>,
> +					<GIC_SPI 255 IRQ_TYPE_LEVEL_LOW>,
> +					<GIC_SPI 256 IRQ_TYPE_LEVEL_LOW>;
> +			interrupt-names = "cam_uni",
> +					"cam_a",
> +					"cam_b",
> +					"cam_c";
> +			iommus = <&iommu M4U_PORT_CAM_IMGO>;
> +			clocks = <&camsys CLK_CAM_CAM>,
> +					<&camsys CLK_CAM_CAMTG>;
> +			clock-names = "camsys_cam_cgpdn",
> +					"camsys_camtg_cgpdn";
> +			mediatek,larb = <&larb3>,
> +					<&larb6>;
> +			power-domains = <&scpsys MT8183_POWER_DOMAIN_CAM>;
> +			mediatek,scp = <&scp>;
> +		};
> -- 
> 2.18.0
> 

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

^ permalink raw reply

* Re: [PATCH v3] arm64: implement KPROBES_ON_FTRACE
From: Naveen N. Rao @ 2019-08-21 20:12 UTC (permalink / raw)
  To: Catalin Marinas, Jonathan Corbet, Jisheng Zhang, Masami Hiramatsu,
	Peter Zijlstra, Thomas Gleixner, Will Deacon
  Cc: Ingo Molnar, Steven Rostedt, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <20190821183501.33588dd8@xhacker.debian>

Jisheng Zhang wrote:
> KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
> eliminates the need for a trap, as well as the need to emulate or
> single-step instructions.
> 
> Tested on berlin arm64 platform.
> 
> ~ # mount -t debugfs debugfs /sys/kernel/debug/
> ~ # cd /sys/kernel/debug/
> /sys/kernel/debug # echo 'p _do_fork' > tracing/kprobe_events
> 
> before the patch:
> 
> /sys/kernel/debug # cat kprobes/list
> ffffff801009fe28  k  _do_fork+0x0    [DISABLED]
> 
> after the patch:
> 
> /sys/kernel/debug # cat kprobes/list
> ffffff801009ff54  k  _do_fork+0x4    [DISABLED][FTRACE]
> 
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
> eliminates the need for a trap, as well as the need to emulate or
> single-step instructions.
> 
> Applied after arm64 FTRACE_WITH_REGS:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2019-August/674404.html
> 
> Changes since v2:
>   - remove patch1, make it a single cleanup patch
>   - remove "This patch" in the change log
>   - implement arm64's kprobe_lookup_name() and arch_kprobe_on_func_entry instead
>     patching the common kprobes code
> 
> Changes since v1:
>   - make the kprobes/x86: use instruction_pointer and instruction_pointer_set
>     as patch1
>   - add Masami's ACK to patch1
>   - add some description about KPROBES_ON_FTRACE and why we need it on
>     arm64
>   - correct the log before the patch
>   - remove the consolidation patch, make it as TODO
>   - only adjust kprobe's addr when KPROBE_FLAG_FTRACE is set
>   - if KPROBES_ON_FTRACE, ftrace_call_adjust() the kprobe's addr before
>     calling ftrace_location()
>   - update the kprobes-on-ftrace/arch-support.txt in doc
> 
> 
>  .../debug/kprobes-on-ftrace/arch-support.txt  |  2 +-
>  arch/arm64/Kconfig                            |  1 +
>  arch/arm64/kernel/probes/Makefile             |  1 +
>  arch/arm64/kernel/probes/ftrace.c             | 60 +++++++++++++++++++
>  arch/arm64/kernel/probes/kprobes.c            | 23 +++++++
>  5 files changed, 86 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/kernel/probes/ftrace.c
> 
> diff --git a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> index 68f266944d5f..e8358a38981c 100644
> --- a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> +++ b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> @@ -9,7 +9,7 @@
>      |       alpha: | TODO |
>      |         arc: | TODO |
>      |         arm: | TODO |
> -    |       arm64: | TODO |
> +    |       arm64: |  ok  |
>      |         c6x: | TODO |
>      |        csky: | TODO |
>      |       h8300: | TODO |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 663392d1eae2..928700f15e23 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -167,6 +167,7 @@ config ARM64
>  	select HAVE_STACKPROTECTOR
>  	select HAVE_SYSCALL_TRACEPOINTS
>  	select HAVE_KPROBES
> +	select HAVE_KPROBES_ON_FTRACE
>  	select HAVE_KRETPROBES
>  	select HAVE_GENERIC_VDSO
>  	select IOMMU_DMA if IOMMU_SUPPORT
> diff --git a/arch/arm64/kernel/probes/Makefile b/arch/arm64/kernel/probes/Makefile
> index 8e4be92e25b1..4020cfc66564 100644
> --- a/arch/arm64/kernel/probes/Makefile
> +++ b/arch/arm64/kernel/probes/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_KPROBES)		+= kprobes.o decode-insn.o	\
>  				   simulate-insn.o
>  obj-$(CONFIG_UPROBES)		+= uprobes.o decode-insn.o	\
>  				   simulate-insn.o
> +obj-$(CONFIG_KPROBES_ON_FTRACE)	+= ftrace.o
> diff --git a/arch/arm64/kernel/probes/ftrace.c b/arch/arm64/kernel/probes/ftrace.c
> new file mode 100644
> index 000000000000..1f0c09d02bb8
> --- /dev/null
> +++ b/arch/arm64/kernel/probes/ftrace.c
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Dynamic Ftrace based Kprobes Optimization
> + *
> + * Copyright (C) Hitachi Ltd., 2012
> + * Copyright (C) 2019 Jisheng Zhang <jszhang@kernel.org>
> + *		      Synaptics Incorporated
> + */
> +
> +#include <linux/kprobes.h>
> +
> +/* Ftrace callback handler for kprobes -- called under preepmt disabed */
> +void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> +			   struct ftrace_ops *ops, struct pt_regs *regs)
> +{
> +	struct kprobe *p;
> +	struct kprobe_ctlblk *kcb;
> +
> +	/* Preempt is disabled by ftrace */
> +	p = get_kprobe((kprobe_opcode_t *)ip);
> +	if (unlikely(!p) || kprobe_disabled(p))
> +		return;
> +
> +	kcb = get_kprobe_ctlblk();
> +	if (kprobe_running()) {
> +		kprobes_inc_nmissed_count(p);
> +	} else {
> +		unsigned long orig_ip = instruction_pointer(regs);
> +		/* Kprobe handler expects regs->pc = pc + 4 as breakpoint hit */
> +		instruction_pointer_set(regs, ip + sizeof(kprobe_opcode_t));
> +
> +		__this_cpu_write(current_kprobe, p);
> +		kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> +		if (!p->pre_handler || !p->pre_handler(p, regs)) {
> +			/*
> +			 * Emulate singlestep (and also recover regs->pc)
> +			 * as if there is a nop
> +			 */
> +			instruction_pointer_set(regs,
> +				(unsigned long)p->addr + MCOUNT_INSN_SIZE);
> +			if (unlikely(p->post_handler)) {
> +				kcb->kprobe_status = KPROBE_HIT_SSDONE;
> +				p->post_handler(p, regs, 0);
> +			}
> +			instruction_pointer_set(regs, orig_ip);
> +		}
> +		/*
> +		 * If pre_handler returns !0, it changes regs->pc. We have to
> +		 * skip emulating post_handler.
> +		 */
> +		__this_cpu_write(current_kprobe, NULL);
> +	}
> +}
> +NOKPROBE_SYMBOL(kprobe_ftrace_handler);
> +
> +int arch_prepare_kprobe_ftrace(struct kprobe *p)
> +{
> +	p->ainsn.api.insn = NULL;
> +	return 0;
> +}
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index c4452827419b..f2bf8c70da79 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -551,6 +551,29 @@ void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
>  	return (void *)orig_ret_address;
>  }
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS

This should be CONFIG_KPROBES_ON_FTRACE since you only want to choose 
the ftrace entry in that case.

> +kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
> +{
> +	unsigned long addr = kallsyms_lookup_name(name);
> +	unsigned long faddr;
> +
> +	/*
> +	 * with -fpatchable-function-entry=2, the first 4 bytes is the
> +	 * LR saver, then the actual call insn. So ftrace location is
> +	 * always on the first 4 bytes offset.
> +	 */
> +	faddr = ftrace_location_range(addr, addr + AARCH64_INSN_SIZE);
> +	if (faddr)
> +		return (kprobe_opcode_t *)faddr;

You should only return the ftrace location if offset is 0, since the 
offset is added to the address returned from here (see _kprobe_addr()).


- Naveen


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

^ permalink raw reply

* [PATCH v3 2/2] arm64: dts: allwinner: a64: Add A64 OlinuXino board (with eMMC)
From: Sunil Mohan Adapa @ 2019-08-21 19:52 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: mark.rutland, maxime.ripard, Sunil Mohan Adapa, Martin Ayotte,
	wens, robh+dt
In-Reply-To: <20190821195217.4166-1-sunil@medhas.org>

A64 OLinuXino board from Olimex has three variants with onboard eMMC:
A64-OLinuXino-1Ge16GW, A64-OLinuXino-1Ge4GW and A64-OLinuXino-2Ge8G-IND. In
addition, there are two variants without eMMC. One without eMMC and one with SPI
flash. This suggests the need for separate device tree for the three eMMC
variants.

This patch has been tested on A64-OLinuXino-1Ge16GW with Linux 5.0 from Debain.
Basic benchmarks using Flexible IO Tester show reasonable performance from the
eMMC.

eMMC - Random Write: 21.3MiB/s
eMMC - Sequential Write: 68.2MiB/s
SD Card - Random Write: 1690KiB/s
SD Card - Sequential Write: 11.0MiB/s

Changes:

  v3: Separate dts for eMMC variants

  v2: Fix descriptions for VCC and VCCQ

Link: https://github.com/armbian/build/commit/174953de1eb09e6aa1ef7075066b573dba625398
Signed-off-by: Martin Ayotte <martinayotte@gmail.com>
[sunil@medhas.org Fix descriptions for VCC and VCCQ, separate dts for eMMC]
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Tested-by: Sunil Mohan Adapa <sunil@medhas.org>
---
 arch/arm64/boot/dts/allwinner/Makefile        |  1 +
 .../allwinner/sun50i-a64-olinuxino-emmc.dts   | 23 +++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino-emmc.dts

diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
index 395fe76f6819..d2418021768b 100644
--- a/arch/arm64/boot/dts/allwinner/Makefile
+++ b/arch/arm64/boot/dts/allwinner/Makefile
@@ -4,6 +4,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-bananapi-m64.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-nanopi-a64.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-oceanic-5205-5inmfd.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-olinuxino.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-olinuxino-emmc.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-orangepi-win.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-lts.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-plus.dtb sun50i-a64-pine64.dtb
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino-emmc.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino-emmc.dts
new file mode 100644
index 000000000000..96ab0227e82d
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino-emmc.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2018 Martin Ayotte <martinayotte@gmail.com>
+ * Copyright (C) 2019 Sunil Mohan Adapa <sunil@medhas.org>
+ */
+
+#include "sun50i-a64-olinuxino.dts"
+
+/ {
+	model = "Olimex A64-Olinuxino-eMMC";
+	compatible = "olimex,a64-olinuxino-emmc", "allwinner,sun50i-a64";
+};
+
+&mmc2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc2_pins>;
+	vmmc-supply = <&reg_dcdc1>;
+	vqmmc-supply = <&reg_dcdc1>;
+	bus-width = <8>;
+	non-removable;
+	cap-mmc-hw-reset;
+	status = "okay";
+};
-- 
2.20.1


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

^ permalink raw reply related

* [PATCH v3 1/2] dt-bindings: arm: sunxi: Add compatible for A64 OlinuXino with eMMC
From: Sunil Mohan Adapa @ 2019-08-21 19:52 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: maxime.ripard, mark.rutland, wens, robh+dt, Sunil Mohan Adapa
In-Reply-To: <20190821195217.4166-1-sunil@medhas.org>

A64 OLinuXino board from Olimex has three variants with onboard eMMC:
A64-OLinuXino-1Ge16GW, A64-OLinuXino-1Ge4GW and A64-OLinuXino-2Ge8G-IND. In
addition, there are two variants without eMMC. One without eMMC and one with SPI
flash. This suggests the need for separate device tree for the three eMMC
variants.

Add new compatible string to the bindings documentation for the A64 OlinuXino
board variant with on-board eMMC.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
---
 Documentation/devicetree/bindings/arm/sunxi.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
index 93dc4c607f07..972b1e9ee804 100644
--- a/Documentation/devicetree/bindings/arm/sunxi.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
@@ -574,6 +574,11 @@ properties:
           - const: olimex,a64-olinuxino
           - const: allwinner,sun50i-a64
 
+      - description: Olimex A64-OlinuXino (with eMMC)
+        items:
+          - const: olimex,a64-olinuxino-emmc
+          - const: allwinner,sun50i-a64
+
       - description: Olimex A64 Teres-I
         items:
           - const: olimex,a64-teres-i
-- 
2.20.1


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

^ 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