Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 11/11] ARM: Allow ARCH_MULTIPLATFORM to be selected for NOMMU
From: Arnd Bergmann @ 2016-11-22 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479806768-39911-12-git-send-email-vladimir.murzin@arm.com>

On Tuesday, November 22, 2016 9:26:08 AM CET Vladimir Murzin wrote:
> With this patch applied potentially any platform can be built in NOMMU
> configurations if CONFIG_EXPERT is selected. However, there is no
> guaranty that platform can successfully run such Image. So the main
> motivation behind of this patch:
> - bring build coverage for NOMMU configurations
> - allow known working NOMMU platforms (like R-class) to be used
> - pave a way to add support for single address space (aka 1:1 mapping)
>   for MMU platforms, so they can be usable in NOMMU configurations
> 
> Cc: Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Ryan Mallon <rmallon@gmail.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>

I'd have to give this a spin with my randconfig build setup, I'd
rather not introduce build regressions. Have you tried an
allmodconfig build with CONFIG_MMU disabled?

Can you provide a git tree that I can try pulling in?

Another question is what architecture levels and what platforms
we want to support without MMU. The only ARMv4/v5 platform we
still have that can actually use NOMMU cores is Integrator
with its ARM7TDMI, ARM920T and ARM966E core tiles (and possibly
others I couldn't immediately find). Do we actually care about
them any more now that all the NOMMU world is ARMv7-M? Are
there any benefits in running an ARM920T or ARM926E core
with MMU disabled, and does this work with your patches?

If not, we could limit it to ARMv7-A/R and possibly ARMv6.
Depending on how the build tests go, a per-platform opt-in
might be easier than having an opt-out for things that
don't work.

	Arnd

^ permalink raw reply

* [PATCH v2] mfd: twl-core: make driver DT only
From: Nicolae Rosia @ 2016-11-22 10:21 UTC (permalink / raw)
  To: linux-arm-kernel

All users are DT-only and it makes no sense to keep
unused code which also prevents further cleanups.

Signed-off-by: Nicolae Rosia <Nicolae_Rosia@mentor.com>
---
Changes in v2:
Increased audience in CC list

 drivers/mfd/Kconfig    |   1 +
 drivers/mfd/twl-core.c | 395 ++-----------------------------------------------
 2 files changed, 10 insertions(+), 386 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c6df644..c180f8b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1333,6 +1333,7 @@ config MFD_TPS80031
 config TWL4030_CORE
 	bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 Support"
 	depends on I2C=y
+	depends on OF
 	select IRQ_DOMAIN
 	select REGMAP_I2C
 	help
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index c64615d..2025326 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -13,6 +13,9 @@
  * Code cleanup and modifications to IRQ handler.
  * by syed khasim <x0khasim@ti.com>
  *
+ * Code cleanup and modifications:
+ * Copyright (C) 2016 Nicolae Rosia <nicolae.rosia@gmail.com>
+ *
  * This program 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
@@ -604,376 +607,6 @@ int twl_get_hfclk_rate(void)
 }
 EXPORT_SYMBOL_GPL(twl_get_hfclk_rate);
 
-static struct device *
-add_numbered_child(unsigned mod_no, const char *name, int num,
-		void *pdata, unsigned pdata_len,
-		bool can_wakeup, int irq0, int irq1)
-{
-	struct platform_device	*pdev;
-	struct twl_client	*twl;
-	int			status, sid;
-
-	if (unlikely(mod_no >= twl_get_last_module())) {
-		pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
-		return ERR_PTR(-EPERM);
-	}
-	sid = twl_priv->twl_map[mod_no].sid;
-	twl = &twl_priv->twl_modules[sid];
-
-	pdev = platform_device_alloc(name, num);
-	if (!pdev)
-		return ERR_PTR(-ENOMEM);
-
-	pdev->dev.parent = &twl->client->dev;
-
-	if (pdata) {
-		status = platform_device_add_data(pdev, pdata, pdata_len);
-		if (status < 0) {
-			dev_dbg(&pdev->dev, "can't add platform_data\n");
-			goto put_device;
-		}
-	}
-
-	if (irq0) {
-		struct resource r[2] = {
-			{ .start = irq0, .flags = IORESOURCE_IRQ, },
-			{ .start = irq1, .flags = IORESOURCE_IRQ, },
-		};
-
-		status = platform_device_add_resources(pdev, r, irq1 ? 2 : 1);
-		if (status < 0) {
-			dev_dbg(&pdev->dev, "can't add irqs\n");
-			goto put_device;
-		}
-	}
-
-	status = platform_device_add(pdev);
-	if (status)
-		goto put_device;
-
-	device_init_wakeup(&pdev->dev, can_wakeup);
-
-	return &pdev->dev;
-
-put_device:
-	platform_device_put(pdev);
-	dev_err(&twl->client->dev, "failed to add device %s\n", name);
-	return ERR_PTR(status);
-}
-
-static inline struct device *add_child(unsigned mod_no, const char *name,
-		void *pdata, unsigned pdata_len,
-		bool can_wakeup, int irq0, int irq1)
-{
-	return add_numbered_child(mod_no, name, -1, pdata, pdata_len,
-		can_wakeup, irq0, irq1);
-}
-
-static struct device *
-add_regulator_linked(int num, struct regulator_init_data *pdata,
-		struct regulator_consumer_supply *consumers,
-		unsigned num_consumers, unsigned long features)
-{
-	struct twl_regulator_driver_data drv_data;
-
-	/* regulator framework demands init_data ... */
-	if (!pdata)
-		return NULL;
-
-	if (consumers) {
-		pdata->consumer_supplies = consumers;
-		pdata->num_consumer_supplies = num_consumers;
-	}
-
-	if (pdata->driver_data) {
-		/* If we have existing drv_data, just add the flags */
-		struct twl_regulator_driver_data *tmp;
-		tmp = pdata->driver_data;
-		tmp->features |= features;
-	} else {
-		/* add new driver data struct, used only during init */
-		drv_data.features = features;
-		drv_data.set_voltage = NULL;
-		drv_data.get_voltage = NULL;
-		drv_data.data = NULL;
-		pdata->driver_data = &drv_data;
-	}
-
-	/* NOTE:  we currently ignore regulator IRQs, e.g. for short circuits */
-	return add_numbered_child(TWL_MODULE_PM_MASTER, "twl_reg", num,
-		pdata, sizeof(*pdata), false, 0, 0);
-}
-
-static struct device *
-add_regulator(int num, struct regulator_init_data *pdata,
-		unsigned long features)
-{
-	return add_regulator_linked(num, pdata, NULL, 0, features);
-}
-
-/*
- * NOTE:  We know the first 8 IRQs after pdata->base_irq are
- * for the PIH, and the next are for the PWR_INT SIH, since
- * that's how twl_init_irq() sets things up.
- */
-
-static int
-add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
-		unsigned long features)
-{
-	struct device	*child;
-
-	if (IS_ENABLED(CONFIG_GPIO_TWL4030) && pdata->gpio) {
-		child = add_child(TWL4030_MODULE_GPIO, "twl4030_gpio",
-				pdata->gpio, sizeof(*pdata->gpio),
-				false, irq_base + GPIO_INTR_OFFSET, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_KEYBOARD_TWL4030) && pdata->keypad) {
-		child = add_child(TWL4030_MODULE_KEYPAD, "twl4030_keypad",
-				pdata->keypad, sizeof(*pdata->keypad),
-				true, irq_base + KEYPAD_INTR_OFFSET, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_TWL4030_MADC) && pdata->madc &&
-	    twl_class_is_4030()) {
-		child = add_child(TWL4030_MODULE_MADC, "twl4030_madc",
-				pdata->madc, sizeof(*pdata->madc),
-				true, irq_base + MADC_INTR_OFFSET, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_RTC_DRV_TWL4030)) {
-		/*
-		 * REVISIT platform_data here currently might expose the
-		 * "msecure" line ... but for now we just expect board
-		 * setup to tell the chip "it's always ok to SET_TIME".
-		 * Eventually, Linux might become more aware of such
-		 * HW security concerns, and "least privilege".
-		 */
-		child = add_child(TWL_MODULE_RTC, "twl_rtc", NULL, 0,
-				true, irq_base + RTC_INTR_OFFSET, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_PWM_TWL)) {
-		child = add_child(TWL_MODULE_PWM, "twl-pwm", NULL, 0,
-				  false, 0, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_PWM_TWL_LED)) {
-		child = add_child(TWL_MODULE_LED, "twl-pwmled", NULL, 0,
-				  false, 0, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_TWL4030_USB) && pdata->usb &&
-	    twl_class_is_4030()) {
-
-		static struct regulator_consumer_supply usb1v5 = {
-			.supply =	"usb1v5",
-		};
-		static struct regulator_consumer_supply usb1v8 = {
-			.supply =	"usb1v8",
-		};
-		static struct regulator_consumer_supply usb3v1 = {
-			.supply =	"usb3v1",
-		};
-
-	/* First add the regulators so that they can be used by transceiver */
-		if (IS_ENABLED(CONFIG_REGULATOR_TWL4030)) {
-			/* this is a template that gets copied */
-			struct regulator_init_data usb_fixed = {
-				.constraints.valid_modes_mask =
-					REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-				.constraints.valid_ops_mask =
-					REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
-			};
-
-			child = add_regulator_linked(TWL4030_REG_VUSB1V5,
-						      &usb_fixed, &usb1v5, 1,
-						      features);
-			if (IS_ERR(child))
-				return PTR_ERR(child);
-
-			child = add_regulator_linked(TWL4030_REG_VUSB1V8,
-						      &usb_fixed, &usb1v8, 1,
-						      features);
-			if (IS_ERR(child))
-				return PTR_ERR(child);
-
-			child = add_regulator_linked(TWL4030_REG_VUSB3V1,
-						      &usb_fixed, &usb3v1, 1,
-						      features);
-			if (IS_ERR(child))
-				return PTR_ERR(child);
-
-		}
-
-		child = add_child(TWL_MODULE_USB, "twl4030_usb",
-				pdata->usb, sizeof(*pdata->usb), true,
-				/* irq0 = USB_PRES, irq1 = USB */
-				irq_base + USB_PRES_INTR_OFFSET,
-				irq_base + USB_INTR_OFFSET);
-
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		/* we need to connect regulators to this transceiver */
-		if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && child) {
-			usb1v5.dev_name = dev_name(child);
-			usb1v8.dev_name = dev_name(child);
-			usb3v1.dev_name = dev_name(child);
-		}
-	}
-
-	if (IS_ENABLED(CONFIG_TWL4030_WATCHDOG) && twl_class_is_4030()) {
-		child = add_child(TWL_MODULE_PM_RECEIVER, "twl4030_wdt", NULL,
-				  0, false, 0, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_INPUT_TWL4030_PWRBUTTON) && twl_class_is_4030()) {
-		child = add_child(TWL_MODULE_PM_MASTER, "twl4030_pwrbutton",
-				  NULL, 0, true, irq_base + 8 + 0, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_MFD_TWL4030_AUDIO) && pdata->audio &&
-	    twl_class_is_4030()) {
-		child = add_child(TWL4030_MODULE_AUDIO_VOICE, "twl4030-audio",
-				pdata->audio, sizeof(*pdata->audio),
-				false, 0, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	/* twl4030 regulators */
-	if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && twl_class_is_4030()) {
-		child = add_regulator(TWL4030_REG_VPLL1, pdata->vpll1,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VIO, pdata->vio,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VDD1, pdata->vdd1,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VDD2, pdata->vdd2,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VMMC1, pdata->vmmc1,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VDAC, pdata->vdac,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator((features & TWL4030_VAUX2)
-					? TWL4030_REG_VAUX2_4030
-					: TWL4030_REG_VAUX2,
-				pdata->vaux2, features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VINTANA1, pdata->vintana1,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VINTANA2, pdata->vintana2,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VINTDIG, pdata->vintdig,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	/* maybe add LDOs that are omitted on cost-reduced parts */
-	if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && !(features & TPS_SUBSET)
-	  && twl_class_is_4030()) {
-		child = add_regulator(TWL4030_REG_VPLL2, pdata->vpll2,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VMMC2, pdata->vmmc2,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VSIM, pdata->vsim,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VAUX1, pdata->vaux1,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VAUX3, pdata->vaux3,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-
-		child = add_regulator(TWL4030_REG_VAUX4, pdata->vaux4,
-					features);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_CHARGER_TWL4030) && pdata->bci &&
-			!(features & (TPS_SUBSET | TWL5031))) {
-		child = add_child(TWL_MODULE_MAIN_CHARGE, "twl4030_bci",
-				pdata->bci, sizeof(*pdata->bci), false,
-				/* irq0 = CHG_PRES, irq1 = BCI */
-				irq_base + BCI_PRES_INTR_OFFSET,
-				irq_base + BCI_INTR_OFFSET);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	if (IS_ENABLED(CONFIG_TWL4030_POWER) && pdata->power) {
-		child = add_child(TWL_MODULE_PM_MASTER, "twl4030_power",
-				  pdata->power, sizeof(*pdata->power), false,
-				  0, 0);
-		if (IS_ERR(child))
-			return PTR_ERR(child);
-	}
-
-	return 0;
-}
-
-/*----------------------------------------------------------------------*/
-
 /*
  * These three functions initialize the on-chip clock framework,
  * letting it generate the right frequencies for USB, MADC, and
@@ -1000,8 +633,7 @@ static inline int __init unprotect_pm_master(void)
 	return e;
 }
 
-static void clocks_init(struct device *dev,
-			struct twl4030_clock_init_data *clock)
+static void clocks_init(struct device *dev)
 {
 	int e = 0;
 	struct clk *osc;
@@ -1031,8 +663,6 @@ static void clocks_init(struct device *dev,
 	}
 
 	ctrl |= HIGH_PERF_SQ;
-	if (clock && clock->ck32k_lowpwr_enable)
-		ctrl |= CK32K_LOWPWR_EN;
 
 	e |= unprotect_pm_master();
 	/* effect->MADC+USB ck en */
@@ -1080,7 +710,6 @@ static struct of_dev_auxdata twl_auxdata_lookup[] = {
 static int
 twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
-	struct twl4030_platform_data	*pdata = dev_get_platdata(&client->dev);
 	struct device_node		*node = client->dev.of_node;
 	struct platform_device		*pdev;
 	const struct regmap_config	*twl_regmap_config;
@@ -1088,8 +717,8 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	int				status;
 	unsigned			i, num_slaves;
 
-	if (!node && !pdata) {
-		dev_err(&client->dev, "no platform data\n");
+	if (!node) {
+		dev_err(&client->dev, "no DT info\n");
 		return -EINVAL;
 	}
 
@@ -1177,7 +806,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	twl_priv->ready = true;
 
 	/* setup clock framework */
-	clocks_init(&pdev->dev, pdata ? pdata->clock : NULL);
+	clocks_init(&pdev->dev);
 
 	/* read TWL IDCODE Register */
 	if (twl_class_is_4030()) {
@@ -1225,14 +854,8 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 				 TWL4030_DCDC_GLOBAL_CFG);
 	}
 
-	if (node) {
-		if (pdata)
-			twl_auxdata_lookup[0].platform_data = pdata->gpio;
-		status = of_platform_populate(node, NULL, twl_auxdata_lookup,
-					      &client->dev);
-	} else {
-		status = add_children(pdata, irq_base, id->driver_data);
-	}
+	status = of_platform_populate(node, NULL, twl_auxdata_lookup,
+					&client->dev);
 
 fail:
 	if (status < 0)
-- 
2.5.5

^ permalink raw reply related

* [PATCH 1/3] ARM: davinci: hawk: fix mmc card detect gpio
From: Sekhar Nori @ 2016-11-22 10:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKXjFTMW7p-TAehV_eTd4nDtC3gvNmcg-e9tLmv4LSX1LEXnhw@mail.gmail.com>

On Tuesday 22 November 2016 03:40 PM, Axel Haslam wrote:
> On Tue, Nov 22, 2016 at 10:53 AM, Sekhar Nori <nsekhar@ti.com> wrote:
>> On Monday 21 November 2016 09:45 PM, Axel Haslam wrote:
>>> The card detect gpio on the hawk board is gpio4_0 and not gpio3_12
>>>
>>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>>
>> The LCDK and HawkBoard are different boards. The HawkBoard schematic
>> from eLinux.org page is broken, but looking for it on the net, I found
>> one and the MMC/SD CD pin in that schematic is indeed connected to GPIO3_12.
>>
>> So I believe the original code is correct.
> 
> mmm, ok, the lcdk is booted using the hawk board file,

I think this is just a coincidence. The LCDK came after the hawkboard as
its replacement, and I guess there was some effort to derive out of
hawkboard design. But they are not 100% software compatible, like you
are discovering now.

> so we should differentiate the two boards?

Yes.

> or should we just live with inconsistencies on the lcdk?

Not sure what you mean by "live with inconsistencies on the lcdk". The
two boards need to be treated as close cousins, but independent boards.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH 1/6] reset: hisilicon: add reset core
From: Philipp Zabel @ 2016-11-22 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479800961-6249-2-git-send-email-zhangfei.gao@linaro.org>

Hi Zhangfei,

Am Dienstag, den 22.11.2016, 15:49 +0800 schrieb Zhangfei Gao:
> reset.c will be shared by hisilicon chips like hi3660 and hi6220
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> ---
>  drivers/reset/Makefile           |   2 +-
>  drivers/reset/hisilicon/Makefile |   1 +
>  drivers/reset/hisilicon/reset.c  | 108 +++++++++++++++++++++++++++++++++++++++
>  drivers/reset/hisilicon/reset.h  |  37 ++++++++++++++
>  4 files changed, 147 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/reset/hisilicon/reset.c
>  create mode 100644 drivers/reset/hisilicon/reset.h
> 
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index bbe7026..7e3dc4e 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,8 +1,8 @@
>  obj-y += core.o
> -obj-y += hisilicon/
>  obj-$(CONFIG_ARCH_STI) += sti/
>  obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
>  obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
> +obj-$(CONFIG_ARCH_HISI) += hisilicon/
>  obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>  obj-$(CONFIG_RESET_MESON) += reset-meson.o
>  obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> diff --git a/drivers/reset/hisilicon/Makefile b/drivers/reset/hisilicon/Makefile
> index c932f86..df511f5 100644
> --- a/drivers/reset/hisilicon/Makefile
> +++ b/drivers/reset/hisilicon/Makefile
> @@ -1 +1,2 @@
> +obj-y	+= reset.o
>  obj-$(CONFIG_COMMON_RESET_HI6220) += hi6220_reset.o
> diff --git a/drivers/reset/hisilicon/reset.c b/drivers/reset/hisilicon/reset.c
> new file mode 100644
> index 0000000..c4971c9
> --- /dev/null
> +++ b/drivers/reset/hisilicon/reset.c
> @@ -0,0 +1,108 @@
> +/*
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program 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.
> + */
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/types.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +
> +#include "reset.h"
> +
> +struct hisi_reset_controller {
> +	struct reset_controller_dev rst;
> +	const struct hisi_reset_channel_data *channels;
> +	struct regmap *map;
> +};
> +
> +#define to_hisi_reset_controller(_rst) \
> +	container_of(_rst, struct hisi_reset_controller, rst)
> +
> +static int hisi_reset_program_hw(struct reset_controller_dev *rcdev,
> +				 unsigned long idx, bool assert)
> +{
> +	struct hisi_reset_controller *rc = to_hisi_reset_controller(rcdev);
> +	const struct hisi_reset_channel_data *ch;
> +
> +	if (idx >= rcdev->nr_resets)
> +		return -EINVAL;
> +
> +	ch = &rc->channels[idx];
> +
> +	if (assert)
> +		return regmap_write(rc->map, ch->enable.reg,
> +				    GENMASK(ch->enable.msb, ch->enable.lsb));
> +	else
> +		return regmap_write(rc->map, ch->disable.reg,
> +				    GENMASK(ch->disable.msb, ch->disable.lsb));

These fields are always 1-bit wide and you are not using the
regmap_field functions to access them, so I'd suggest to remove the
struct reg_field indirection and overhead and just write

	if (assert)
		return regmap_write(rc->map, ch->enable_reg, ch->bit);
	else
		return regmap_write(rc->map, ch->disable_reg, ch->bit);

here.

> +}
> +
> +static int hisi_reset_assert(struct reset_controller_dev *rcdev,
> +			     unsigned long idx)
> +{
> +	return hisi_reset_program_hw(rcdev, idx, true);
> +}
> +
> +static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
> +			       unsigned long idx)
> +{
> +	return hisi_reset_program_hw(rcdev, idx, false);
> +}
> +
> +static int hisi_reset_dev(struct reset_controller_dev *rcdev,
> +			  unsigned long idx)
> +{
> +	int err;
> +
> +	err = hisi_reset_assert(rcdev, idx);
> +	if (err)
> +		return err;
> +
> +	return hisi_reset_deassert(rcdev, idx);
> +}
> +
> +static struct reset_control_ops hisi_reset_ops = {
> +	.reset    = hisi_reset_dev,
> +	.assert   = hisi_reset_assert,
> +	.deassert = hisi_reset_deassert,
> +};
> +
> +int hisi_reset_probe(struct platform_device *pdev)
> +{
> +	struct hisi_reset_controller *rc;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct hisi_reset_controller_data *d;
> +	struct device *dev = &pdev->dev;
> +	const struct of_device_id *match;
> +
> +	match = of_match_device(dev->driver->of_match_table, dev);
> +	if (!match || !match->data)
> +		return -EINVAL;
> +
> +	d = (struct hisi_reset_controller_data *)match->data;
> +	rc = devm_kzalloc(dev, sizeof(*rc), GFP_KERNEL);
> +	if (!rc)
> +		return -ENOMEM;
> +
> +	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");
> +	if (IS_ERR(rc->map)) {
> +		dev_err(dev, "failed to get hisi,rst-syscon\n");
> +		return PTR_ERR(rc->map);
> +	}
> +
> +	rc->rst.ops = &hisi_reset_ops,
> +	rc->rst.of_node = np;
> +	rc->rst.nr_resets = d->nr_channels;
> +	rc->channels = d->channels;
> +
> +	return reset_controller_register(&rc->rst);
> +}
> +EXPORT_SYMBOL_GPL(hisi_reset_probe);
> diff --git a/drivers/reset/hisilicon/reset.h b/drivers/reset/hisilicon/reset.h
> new file mode 100644
> index 0000000..77259ee
> --- /dev/null
> +++ b/drivers/reset/hisilicon/reset.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program 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.
> + */
> +
> +#ifndef __HISILICON_RESET_H
> +#define __HISILICON_RESET_H
> +
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +
> +/* reset separated register offset is 0x4 */
> +#define HISI_RST_SEP(off, bit)					\
> +	{ .enable	= REG_FIELD(off, bit, bit),		\
> +	  .disable	= REG_FIELD(off + 0x4, bit, bit),	\
> +	  .status	= REG_FIELD(off + 0x8, bit, bit), }
> +
> +struct hisi_reset_channel_data {
> +	struct reg_field enable;
> +	struct reg_field disable;
> +	struct reg_field status;
> +};

Are you expecting the bits to be at different positions in the
enable/disable/status registers? How about just

#define HISI_RST_SEP(off, _bit)		\
	{ .enable_reg	= (off),	\
	  .disable_reg	= (off) + 0x4,	\
	  .status_reg	= (off) + 0x8,	\
	  .bit		= (_bit), }

struct hisi_reset_channel_data {
	unsigned int enable_reg;
	unsigned int disable_reg;
	unsigned int status_reg;
	unsigned int bit;
};

as those struct reg_field are not accessed via regmap_field_* functions
anyway.

> +
> +struct hisi_reset_controller_data {
> +	int nr_channels;
> +	const struct hisi_reset_channel_data *channels;
> +};
> +
> +int hisi_reset_probe(struct platform_device *pdev);
> +
> +#endif /* __HISILICON_RESET_H */

regards
Philipp

^ permalink raw reply

* [PATCH 1/6] reset: hisilicon: add reset core
From: Philipp Zabel @ 2016-11-22 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479800961-6249-2-git-send-email-zhangfei.gao@linaro.org>

Hi Zhangfei,

Am Dienstag, den 22.11.2016, 15:49 +0800 schrieb Zhangfei Gao:
> reset.c will be shared by hisilicon chips like hi3660 and hi6220
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> ---
>  drivers/reset/Makefile           |   2 +-
>  drivers/reset/hisilicon/Makefile |   1 +
>  drivers/reset/hisilicon/reset.c  | 108 +++++++++++++++++++++++++++++++++++++++
>  drivers/reset/hisilicon/reset.h  |  37 ++++++++++++++
>  4 files changed, 147 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/reset/hisilicon/reset.c
>  create mode 100644 drivers/reset/hisilicon/reset.h
> 
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index bbe7026..7e3dc4e 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,8 +1,8 @@
>  obj-y += core.o
> -obj-y += hisilicon/
>  obj-$(CONFIG_ARCH_STI) += sti/
>  obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
>  obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
> +obj-$(CONFIG_ARCH_HISI) += hisilicon/
>  obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>  obj-$(CONFIG_RESET_MESON) += reset-meson.o
>  obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> diff --git a/drivers/reset/hisilicon/Makefile b/drivers/reset/hisilicon/Makefile
> index c932f86..df511f5 100644
> --- a/drivers/reset/hisilicon/Makefile
> +++ b/drivers/reset/hisilicon/Makefile
> @@ -1 +1,2 @@
> +obj-y	+= reset.o
>  obj-$(CONFIG_COMMON_RESET_HI6220) += hi6220_reset.o
> diff --git a/drivers/reset/hisilicon/reset.c b/drivers/reset/hisilicon/reset.c
> new file mode 100644
> index 0000000..c4971c9
> --- /dev/null
> +++ b/drivers/reset/hisilicon/reset.c
> @@ -0,0 +1,108 @@
> +/*
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program 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.
> + */
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/types.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +
> +#include "reset.h"
> +
> +struct hisi_reset_controller {
> +	struct reset_controller_dev rst;
> +	const struct hisi_reset_channel_data *channels;
> +	struct regmap *map;
> +};
> +
> +#define to_hisi_reset_controller(_rst) \
> +	container_of(_rst, struct hisi_reset_controller, rst)
> +
> +static int hisi_reset_program_hw(struct reset_controller_dev *rcdev,
> +				 unsigned long idx, bool assert)
> +{
> +	struct hisi_reset_controller *rc = to_hisi_reset_controller(rcdev);
> +	const struct hisi_reset_channel_data *ch;
> +
> +	if (idx >= rcdev->nr_resets)
> +		return -EINVAL;
> +
> +	ch = &rc->channels[idx];
> +
> +	if (assert)
> +		return regmap_write(rc->map, ch->enable.reg,
> +				    GENMASK(ch->enable.msb, ch->enable.lsb));
> +	else
> +		return regmap_write(rc->map, ch->disable.reg,
> +				    GENMASK(ch->disable.msb, ch->disable.lsb));

These fields are always 1-bit wide and you are not using the
regmap_field functions to access them, so I'd suggest to remove the
struct reg_field indirection and overhead and just write

	if (assert)
		return regmap_write(rc->map, ch->enable_reg, ch->bit);
	else
		return regmap_write(rc->map, ch->disable_reg, ch->bit);

here.

> +}
> +
> +static int hisi_reset_assert(struct reset_controller_dev *rcdev,
> +			     unsigned long idx)
> +{
> +	return hisi_reset_program_hw(rcdev, idx, true);
> +}
> +
> +static int hisi_reset_deassert(struct reset_controller_dev *rcdev,
> +			       unsigned long idx)
> +{
> +	return hisi_reset_program_hw(rcdev, idx, false);
> +}
> +
> +static int hisi_reset_dev(struct reset_controller_dev *rcdev,
> +			  unsigned long idx)
> +{
> +	int err;
> +
> +	err = hisi_reset_assert(rcdev, idx);
> +	if (err)
> +		return err;
> +
> +	return hisi_reset_deassert(rcdev, idx);
> +}
> +
> +static struct reset_control_ops hisi_reset_ops = {
> +	.reset    = hisi_reset_dev,
> +	.assert   = hisi_reset_assert,
> +	.deassert = hisi_reset_deassert,
> +};
> +
> +int hisi_reset_probe(struct platform_device *pdev)
> +{
> +	struct hisi_reset_controller *rc;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct hisi_reset_controller_data *d;
> +	struct device *dev = &pdev->dev;
> +	const struct of_device_id *match;
> +
> +	match = of_match_device(dev->driver->of_match_table, dev);
> +	if (!match || !match->data)
> +		return -EINVAL;
> +
> +	d = (struct hisi_reset_controller_data *)match->data;
> +	rc = devm_kzalloc(dev, sizeof(*rc), GFP_KERNEL);
> +	if (!rc)
> +		return -ENOMEM;
> +
> +	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");
> +	if (IS_ERR(rc->map)) {
> +		dev_err(dev, "failed to get hisi,rst-syscon\n");
> +		return PTR_ERR(rc->map);
> +	}
> +
> +	rc->rst.ops = &hisi_reset_ops,
> +	rc->rst.of_node = np;
> +	rc->rst.nr_resets = d->nr_channels;
> +	rc->channels = d->channels;
> +
> +	return reset_controller_register(&rc->rst);
> +}
> +EXPORT_SYMBOL_GPL(hisi_reset_probe);
> diff --git a/drivers/reset/hisilicon/reset.h b/drivers/reset/hisilicon/reset.h
> new file mode 100644
> index 0000000..77259ee
> --- /dev/null
> +++ b/drivers/reset/hisilicon/reset.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (c) 2016-2017 Linaro Ltd.
> + * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
> + *
> + * This program 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.
> + */
> +
> +#ifndef __HISILICON_RESET_H
> +#define __HISILICON_RESET_H
> +
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +
> +/* reset separated register offset is 0x4 */
> +#define HISI_RST_SEP(off, bit)					\
> +	{ .enable	= REG_FIELD(off, bit, bit),		\
> +	  .disable	= REG_FIELD(off + 0x4, bit, bit),	\
> +	  .status	= REG_FIELD(off + 0x8, bit, bit), }
> +
> +struct hisi_reset_channel_data {
> +	struct reg_field enable;
> +	struct reg_field disable;
> +	struct reg_field status;
> +};

Are you expecting the bits to be at different positions in the
enable/disable/status registers? How about just

#define HISI_RST_SEP(off, _bit)		\
	{ .enable_reg	= (off),	\
	  .disable_reg	= (off) + 0x4,	\
	  .status_reg	= (off) + 0x8,	\
	  .bit		= (_bit), }

struct hisi_reset_channel_data {
	unsigned int enable_reg;
	unsigned int disable_reg;
	unsigned int status_reg;
	unsigned int bit;
};

as those struct reg_field are not accessed via regmap_field_* functions
anyway.

> +
> +struct hisi_reset_controller_data {
> +	int nr_channels;
> +	const struct hisi_reset_channel_data *channels;
> +};
> +
> +int hisi_reset_probe(struct platform_device *pdev);
> +
> +#endif /* __HISILICON_RESET_H */

regards
Philipp

^ permalink raw reply

* [PATCH 3/6] reset: hisilicon: add reset-hi3660
From: Philipp Zabel @ 2016-11-22 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3166877.sQekoU5ezv@wuerfel>

Am Dienstag, den 22.11.2016, 10:42 +0100 schrieb Arnd Bergmann:
> On Tuesday, November 22, 2016 5:34:05 PM CET zhangfei wrote:
> > On 2016?11?22? 16:50, Arnd Bergmann wrote:
> > > On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:
> > >> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {
> > >> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),
> > >> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),
> > >> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),
> > >> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),
> > >> +};
> > >> +
> > >> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {
> > >> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),
> > >> +       .channels = hi3660_iomcu_rst,
> > >> +};
> > >> +
> > >> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {
> > >> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),
> > >> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),
> > >> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),
> > >> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),
> > >> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),
> > >> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),
> > >> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),
> > >> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),
> > >> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),
> > >> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),
> > >> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),
> > >> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),
> > >> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),
> > >> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),
> > >> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),
> > >> +};
> > > I think you can avoid the trap of the ABI incompatibility if
> > > you just define those as in the binding as tuples, using #reset-cells=2.
> > >
> > > In particular for the first set, it seems really silly to redefine
> > > the numbers when there is just a simple integer number.
> > 
> > Could you clarify more, still not understand.
> > The number is index of the arrays, and the index will be used in dts.
> > The arrays lists the registers offset and bit shift.
> > For example:
> > 
> > [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.
> > 
> > And Documentation/devicetree/bindings/reset/reset.txt
> > Required properties:
> > #reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes
> >                  with a single reset output and 1 for nodes with multiple
> >                  reset outputs.

This is just a suggestion, for reset controllers where the reset lines
can reasonably be enumerated by a single integer. If there is a good
reason to use more complicated bindings, more cells can be used.
That being said, I dislike having to spread register/bit information
throughout the device trees at the consumer/phandle sites, if the
register/bit information absolutely has to be put into the device tree,
I'd prefer a binding similar to ti-syscon, where it's all in one place.

> You can easily enumerate the registers that contain reset bits here,
> so just use one cell for the register and another one for the index.

Changing the reset cells is an incompatible change, and this is not a
straight forward register/bit mapping in hardware either. There are
currently three registers involved: enable (+0x0), disable (+0x4), and
status (+0x8). Also, what if in the future one of these reset bits have
to be handled inverted (as just happened for hi3519)?

regards
Philipp

^ permalink raw reply

* [PATCH 3/3] ARM: davinci: hawk: use gpio descriptor for card detect
From: Sekhar Nori @ 2016-11-22 10:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121161541.27048-4-ahaslam@baylibre.com>

On Monday 21 November 2016 09:45 PM, Axel Haslam wrote:
> Currently the mmc driver is polling the gpio to know if the
> card was removed.
> 
> By using a gpio descriptor instead of the platform callbacks, the
> driver will be able to register the gpio with the mmc core with API's
> designed for this purpose.
> 
> This has the advantage that an irq will be registered,
> and polling is no longer needed. Also, platform callbacks can be removed.
> 
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>

This patch looks good, provided it is not based of 1/3 and 2/3. There
are other boards in mach-davinci using the card detect and wp callbacks.
And some like board-dm365-evm.c have those pins routed through a CPLD.
So I guess there is more work to be done before platform callbacks can
completely be removed from MMC/SD driver. But the closer we get, the
better it is :)

Thanks,
Sekhar

^ permalink raw reply

* [PATCH] ARM: dts: da850: specify max width for display node
From: Tomi Valkeinen @ 2016-11-22 10:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479807775-28283-1-git-send-email-bgolaszewski@baylibre.com>

On 22/11/16 11:42, Bartosz Golaszewski wrote:
> It has been determined that the highest resolution supported correctly
> by LCDC rev1 is 800x600 on da850 due to memory bandwidth constraints.
> 
> Set the max_width property in da850.dtsi to 800.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 36066fa..0876238 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -441,6 +441,7 @@
>  			compatible = "ti,da850-tilcdc";
>  			reg = <0x213000 0x1000>;
>  			interrupts = <52>;
> +			max-width = <800>;
>  			status = "disabled";
>  		};
>  	};
> 

Does 1024x768 at 10 work?

The max-width should be the hardware's maximum supported width, not used
for bandwidth. tilcdc has max-bandwidth property for that.

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161122/6e711402/attachment.sig>

^ permalink raw reply

* [PATCH] ARM: dts: da850: specify max width for display node
From: Bartosz Golaszewski @ 2016-11-22 10:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <77c993f3-1b5f-94cf-c4bc-8a30148e0a4d@ti.com>

2016-11-22 11:27 GMT+01:00 Tomi Valkeinen <tomi.valkeinen@ti.com>:
> On 22/11/16 11:42, Bartosz Golaszewski wrote:
>> It has been determined that the highest resolution supported correctly
>> by LCDC rev1 is 800x600 on da850 due to memory bandwidth constraints.
>>
>> Set the max_width property in da850.dtsi to 800.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  arch/arm/boot/dts/da850.dtsi | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 36066fa..0876238 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -441,6 +441,7 @@
>>                       compatible = "ti,da850-tilcdc";
>>                       reg = <0x213000 0x1000>;
>>                       interrupts = <52>;
>> +                     max-width = <800>;
>>                       status = "disabled";
>>               };
>>       };
>>
>
> Does 1024x768 at 10 work?
>
> The max-width should be the hardware's maximum supported width, not used
> for bandwidth. tilcdc has max-bandwidth property for that.
>
>  Tomi
>

Eeek I misread Jyri's answer.

Will fix that in v2.

Thanks,
Bartosz

^ permalink raw reply

* [RESEND PATCH 1/3] ARM: davinci: da830: Handle vbus with a regulator
From: Sekhar Nori @ 2016-11-22 10:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121165349.29128-2-ahaslam@baylibre.com>

On Monday 21 November 2016 10:23 PM, Axel Haslam wrote:

>  static __init void da830_evm_usb_init(void)
>  {
> @@ -145,23 +123,13 @@ static __init void da830_evm_usb_init(void)
>  		return;
>  	}
>  
> -	ret = gpio_request(ON_BD_USB_DRV, "ON_BD_USB_DRV");
> -	if (ret) {
> -		pr_err("%s: failed to request GPIO for USB 1.1 port power control: %d\n",
> -		       __func__, ret);
> -		return;
> -	}
> -	gpio_direction_output(ON_BD_USB_DRV, 0);
> +	gpiod_add_lookup_table(&usb11_gpios_table);
>  
> -	ret = gpio_request(ON_BD_USB_OVC, "ON_BD_USB_OVC");
> -	if (ret) {
> -		pr_err("%s: failed to request GPIO for USB 1.1 port over-current indicator: %d\n",
> -		       __func__, ret);
> -		return;
> -	}
> -	gpio_direction_input(ON_BD_USB_OVC);
> +	ret = platform_device_register(&da8xx_usb11_regulator);
> +	if (ret)
> +		pr_warn("fail to add ohci regulator\n");

Can you follow the same style as used in other error/warning messages in
this function (adding a function name prefix, printing of error code).

Thanks,
Sekhar

^ permalink raw reply

* [RESEND PATCH 2/3] ARM: davinci: hawk: Remove vbus and over current gpios
From: Sekhar Nori @ 2016-11-22 10:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121165349.29128-3-ahaslam@baylibre.com>

On Monday 21 November 2016 10:23 PM, Axel Haslam wrote:
> The hawk board VBUS is fixed to a 5v source, and the over
> current pin is actually not connected to the SoC.
> 
> Do not reseve these gpios for OHCI as they are not related
> to usb.
> 
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>

As discussed over the MMC/SD patches, this patch should be based off the
hawkboard schematic, not the LCDK schematic.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH 0/3] ARM: da8xx: fix section mismatch in new drivers
From: Bartosz Golaszewski @ 2016-11-22 10:41 UTC (permalink / raw)
  To: linux-arm-kernel

Sekhar noticed there's a section mismatch in the da8xx-mstpri and
da8xx-ddrctl drivers. This is caused by calling
of_flat_dt_get_machine_name() which has an __init annotation.

This series addresses this issue by introducing a new function that
allows to retrieve the compatible property of the root node and
using it instead of of_flat_dt_get_machine_name() in the new drivers.

Bartosz Golaszewski (3):
  of: base: add support to get machine compatible string
  bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name()
  memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name()

 drivers/bus/da8xx-mstpri.c    |  2 +-
 drivers/memory/da8xx-ddrctl.c |  2 +-
 drivers/of/base.c             | 22 ++++++++++++++++++++++
 include/linux/of.h            |  6 ++++++
 4 files changed, 30 insertions(+), 2 deletions(-)

-- 
2.9.3

^ permalink raw reply

* [PATCH 1/3] of: base: add support to get machine compatible string
From: Bartosz Golaszewski @ 2016-11-22 10:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479811311-3080-1-git-send-email-bgolaszewski@baylibre.com>

Add a function allowing to retrieve the compatible string of the root
node of the device tree.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/of/base.c  | 22 ++++++++++++++++++++++
 include/linux/of.h |  6 ++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index a0bccb5..bbfe5e9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -546,6 +546,28 @@ int of_machine_is_compatible(const char *compat)
 EXPORT_SYMBOL(of_machine_is_compatible);
 
 /**
+ * of_machine_get_compatible - Get the compatible property of the root node
+ *
+ * Returns a NULL-terminated string containing the compatible if it could
+ * be found, NULL otherwise.
+ */
+const char *of_machine_get_compatible(void)
+{
+	struct device_node *root;
+	const char *compatible;
+	int ret = -1;
+
+	root = of_find_node_by_path("/");
+	if (root) {
+		ret = of_property_read_string(root, "compatible", &compatible);
+		of_node_put(root);
+	}
+
+	return ret ? NULL : compatible;
+}
+EXPORT_SYMBOL(of_machine_get_compatible);
+
+/**
  *  __of_device_is_available - check if a device is available for use
  *
  *  @device: Node to check for availability, with locks already held
diff --git a/include/linux/of.h b/include/linux/of.h
index 299aeb1..664b734 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -367,6 +367,7 @@ extern int of_alias_get_id(struct device_node *np, const char *stem);
 extern int of_alias_get_highest_id(const char *stem);
 
 extern int of_machine_is_compatible(const char *compat);
+extern const char *of_machine_get_compatible(void);
 
 extern int of_add_property(struct device_node *np, struct property *prop);
 extern int of_remove_property(struct device_node *np, struct property *prop);
@@ -788,6 +789,11 @@ static inline int of_machine_is_compatible(const char *compat)
 	return 0;
 }
 
+static inline const char *of_machine_get_compatible(void)
+{
+	return NULL;
+}
+
 static inline bool of_console_check(const struct device_node *dn, const char *name, int index)
 {
 	return false;
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/3] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name()
From: Bartosz Golaszewski @ 2016-11-22 10:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479811311-3080-1-git-send-email-bgolaszewski@baylibre.com>

In order to avoid a section mismatch use of_machine_get_compatible()
instead of of_flat_dt_get_machine_name() when printing the error
message.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/bus/da8xx-mstpri.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/da8xx-mstpri.c b/drivers/bus/da8xx-mstpri.c
index 85f0b53..41cbbe6 100644
--- a/drivers/bus/da8xx-mstpri.c
+++ b/drivers/bus/da8xx-mstpri.c
@@ -227,7 +227,7 @@ static int da8xx_mstpri_probe(struct platform_device *pdev)
 	prio_list = da8xx_mstpri_get_board_prio();
 	if (!prio_list) {
 		dev_err(dev, "no master priotities defined for board '%s'\n",
-			of_flat_dt_get_machine_name());
+			of_machine_get_compatible());
 		return -EINVAL;
 	}
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH 3/3] memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name()
From: Bartosz Golaszewski @ 2016-11-22 10:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479811311-3080-1-git-send-email-bgolaszewski@baylibre.com>

In order to avoid a section mismatch use of_machine_get_compatible()
instead of of_flat_dt_get_machine_name() when printing the error
message.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/memory/da8xx-ddrctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
index a20e7bb..179f505 100644
--- a/drivers/memory/da8xx-ddrctl.c
+++ b/drivers/memory/da8xx-ddrctl.c
@@ -118,7 +118,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
 	setting = da8xx_ddrctl_get_board_settings();
 	if (!setting) {
 		dev_err(dev, "no settings for board '%s'\n",
-			of_flat_dt_get_machine_name());
+			of_machine_get_compatible());
 		return -EINVAL;
 	}
 
-- 
2.9.3

^ permalink raw reply related

* [RESEND PATCH 3/3] ARM: davinci: remove ohci platform usage
From: Sekhar Nori @ 2016-11-22 10:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121165349.29128-4-ahaslam@baylibre.com>

On Monday 21 November 2016 10:23 PM, Axel Haslam wrote:
> As all users of ohci platform data have been converted
> to use a regulator, we dont need to pass platform
> data to register the ohci device anymore.
> 
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>

This looks good to me.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH v2] mfd: twl-core: make driver DT only
From: Baruch Siach @ 2016-11-22 10:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479810090-12183-1-git-send-email-Nicolae_Rosia@mentor.com>

Hi Nicolae,

On Tue, Nov 22, 2016 at 12:21:30PM +0200, Nicolae Rosia wrote:
> All users are DT-only and it makes no sense to keep
> unused code which also prevents further cleanups.
> 
> Signed-off-by: Nicolae Rosia <Nicolae_Rosia@mentor.com>
> ---
> Changes in v2:
> Increased audience in CC list
> 
>  drivers/mfd/Kconfig    |   1 +
>  drivers/mfd/twl-core.c | 395 ++-----------------------------------------------
>  2 files changed, 10 insertions(+), 386 deletions(-)

[...]

> @@ -1080,7 +710,6 @@ static struct of_dev_auxdata twl_auxdata_lookup[] = {
>  static int
>  twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  {
> -	struct twl4030_platform_data	*pdata = dev_get_platdata(&client->dev);

Can struct twl4030_platform_data declaration be removed now from 
include/linux/i2c/twl.h?

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply

* [PATCH 8/9] mtd: nand: mxc: implement onfi get/set features
From: Sascha Hauer @ 2016-11-22 10:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121145643.7804e28f@bbrezillon>

Hi Boris,

On Mon, Nov 21, 2016 at 02:56:43PM +0100, Boris Brezillon wrote:
> Hi Sascha,
> 
> On Thu, 15 Sep 2016 10:32:52 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > To be able to support different ONFI timing modes we have to implement
> > the onfi_set_features and onfi_get_features. Tested on an i.MX25 SoC.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mtd/nand/mxc_nand.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> > index 5173fad..1db8299 100644
> > --- a/drivers/mtd/nand/mxc_nand.c
> > +++ b/drivers/mtd/nand/mxc_nand.c
> > @@ -1239,6 +1239,57 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
> >  	}
> >  }
> >  
> > +static int mxc_nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
> > +			int addr, uint8_t *subfeature_param)
> > +{
> > +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> > +	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
> > +	int i;
> > +
> > +	if (!chip->onfi_version ||
> > +	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> > +	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> > +		return -EINVAL;
> > +
> > +	host->buf_start = 0;
> > +
> > +	for (i = 0; i < ONFI_SUBFEATURE_PARAM_LEN; ++i)
> > +		chip->write_byte(mtd, subfeature_param[i]);
> > +
> > +	memcpy32_toio(host->main_area0, host->data_buf, mtd->writesize);
> > +	host->devtype_data->send_cmd(host, NAND_CMD_SET_FEATURES, false);
> > +	mxc_do_addr_cycle(mtd, addr, -1);
> > +	host->devtype_data->send_page(mtd, NFC_INPUT);
> 
> I've been working with an mx27 board embedding a NAND device lately,
> and had a closer look at the NAND controller IP.
> With this IP, you're not able to send only 4 bytes of data, and I'm
> sure sure what you're doing here (sending a full page of data) works
> for a SET_FEATURE command.
> 
> Do you have a way to test it (my NAND is not ONFI compliant)? By test
> it, I mean, set a timing mode using SET_FEATURE and check if the new
> mode has been applied using GET_FEATURE.

I have an i.MX27 board with ONFI flash, but this one does not have the
ONFI_OPT_CMD_SET_GET_FEATURES bit set, so I can't test it there.
However, I can confirm that it works on an i.MX25. With the attached
patch applied on vanilla v4.9-rc5 I get:

GET FEATURES. chip->onfi_timing_mode_default: 4
timing before: 0x00
timing after: 0x04

Sascha

-------------------------8<-------------------------------

>From 38e45851a796ba47e25c72ebc58e358c70e18275 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 22 Nov 2016 11:48:36 +0100
Subject: [PATCH] wip

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/nand_base.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 3bde96a..5b5be2b 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1017,12 +1017,24 @@ static int nand_setup_data_interface(struct nand_chip *chip)
 		u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
 			chip->onfi_timing_mode_default,
 		};
+		u8 rmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {};
+
+		printk("GET FEATURES. chip->onfi_timing_mode_default: %d\n", chip->onfi_timing_mode_default);
+		ret = chip->onfi_get_features(mtd, chip, ONFI_FEATURE_ADDR_TIMING_MODE, rmode_param);
+		if (ret)
+			goto err;
+		printk("timing before: 0x%02x\n", rmode_param[0]);
 
 		ret = chip->onfi_set_features(mtd, chip,
 				ONFI_FEATURE_ADDR_TIMING_MODE,
 				tmode_param);
 		if (ret)
 			goto err;
+
+		ret = chip->onfi_get_features(mtd, chip, ONFI_FEATURE_ADDR_TIMING_MODE, rmode_param);
+		if (ret)
+			goto err;
+		printk("timing after: 0x%02x\n", rmode_param[0]);
 	}
 
 	ret = chip->setup_data_interface(mtd, chip->data_interface, false);
-- 
2.10.2

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply related

* [PATCH 1/3] of: base: add support to get machine compatible string
From: Sudeep Holla @ 2016-11-22 10:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479811311-3080-2-git-send-email-bgolaszewski@baylibre.com>



On 22/11/16 10:41, Bartosz Golaszewski wrote:
> Add a function allowing to retrieve the compatible string of the root
> node of the device tree.
>

Rob has queued [1] and it's in -next today. You can reuse that if you
are planning to target this for v4.11 or just use open coding in your
driver for v4.10 and target this move for v4.11 to avoid cross tree
dependencies as I already mentioned in your previous thread.

-- 
Regards,
Sudeep

[1] http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1274549.html

^ permalink raw reply

* [PATCH v2] mfd: twl-core: make driver DT only
From: Nicolae Rosia @ 2016-11-22 10:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161122104510.iujd5revibqgbldl@tarshish>

Hi,

On Tue, Nov 22, 2016 at 12:45 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Nicolae,
>
>
> Can struct twl4030_platform_data declaration be removed now from
> include/linux/i2c/twl.h?
>
> baruch
>
Yes, I have the patches ready and plan on doing that in the next cycle.
I have been sending multiple patches to convert twl related drivers to
DT only and I don't want to create merge conflicts on twl.h

Thanks,
Nicolae

^ permalink raw reply

* [PATCH 1/3] of: base: add support to get machine compatible string
From: Bartosz Golaszewski @ 2016-11-22 10:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5ce9fb9f-459a-562b-2e9f-85d35f9ec035@arm.com>

2016-11-22 11:53 GMT+01:00 Sudeep Holla <sudeep.holla@arm.com>:
>
>
> On 22/11/16 10:41, Bartosz Golaszewski wrote:
>>
>> Add a function allowing to retrieve the compatible string of the root
>> node of the device tree.
>>
>
> Rob has queued [1] and it's in -next today. You can reuse that if you
> are planning to target this for v4.11 or just use open coding in your
> driver for v4.10 and target this move for v4.11 to avoid cross tree
> dependencies as I already mentioned in your previous thread.
>
> --
> Regards,
> Sudeep
>
> [1] http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1274549.html

Rob's patch checks the model first - I'm not sure this is the behavior
we want here as Sekhar suggested we print the machine compatible.

Thanks,
Bartosz Golaszewski

^ permalink raw reply

* [PATCH 1/3] of: base: add support to get machine compatible string
From: Bartosz Golaszewski @ 2016-11-22 10:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMpxmJU=wR4mdQr5tpNSv8POirtWXR=_s7D_Q_H=Cuy=W=-Jtw@mail.gmail.com>

2016-11-22 11:57 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>:
> 2016-11-22 11:53 GMT+01:00 Sudeep Holla <sudeep.holla@arm.com>:
>>
>>
>> On 22/11/16 10:41, Bartosz Golaszewski wrote:
>>>
>>> Add a function allowing to retrieve the compatible string of the root
>>> node of the device tree.
>>>
>>
>> Rob has queued [1] and it's in -next today. You can reuse that if you
>> are planning to target this for v4.11 or just use open coding in your
>> driver for v4.10 and target this move for v4.11 to avoid cross tree
>> dependencies as I already mentioned in your previous thread.
>>
>> --
>> Regards,
>> Sudeep
>>
>> [1] http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1274549.html
>
> Rob's patch checks the model first - I'm not sure this is the behavior
> we want here as Sekhar suggested we print the machine compatible.
>

I meant your patch of course.

Thanks,
Bartosz

^ permalink raw reply

* [PATCH 8/9] mtd: nand: mxc: implement onfi get/set features
From: Boris Brezillon @ 2016-11-22 11:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161122105253.tzz2hvzhkse7telr@pengutronix.de>

On Tue, 22 Nov 2016 11:52:53 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi Boris,
> 
> On Mon, Nov 21, 2016 at 02:56:43PM +0100, Boris Brezillon wrote:
> > Hi Sascha,
> > 
> > On Thu, 15 Sep 2016 10:32:52 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >   
> > > To be able to support different ONFI timing modes we have to implement
> > > the onfi_set_features and onfi_get_features. Tested on an i.MX25 SoC.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > >  drivers/mtd/nand/mxc_nand.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 53 insertions(+)
> > > 
> > > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> > > index 5173fad..1db8299 100644
> > > --- a/drivers/mtd/nand/mxc_nand.c
> > > +++ b/drivers/mtd/nand/mxc_nand.c
> > > @@ -1239,6 +1239,57 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
> > >  	}
> > >  }
> > >  
> > > +static int mxc_nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
> > > +			int addr, uint8_t *subfeature_param)
> > > +{
> > > +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> > > +	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
> > > +	int i;
> > > +
> > > +	if (!chip->onfi_version ||
> > > +	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> > > +	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> > > +		return -EINVAL;
> > > +
> > > +	host->buf_start = 0;
> > > +
> > > +	for (i = 0; i < ONFI_SUBFEATURE_PARAM_LEN; ++i)
> > > +		chip->write_byte(mtd, subfeature_param[i]);
> > > +
> > > +	memcpy32_toio(host->main_area0, host->data_buf, mtd->writesize);
> > > +	host->devtype_data->send_cmd(host, NAND_CMD_SET_FEATURES, false);
> > > +	mxc_do_addr_cycle(mtd, addr, -1);
> > > +	host->devtype_data->send_page(mtd, NFC_INPUT);  
> > 
> > I've been working with an mx27 board embedding a NAND device lately,
> > and had a closer look at the NAND controller IP.
> > With this IP, you're not able to send only 4 bytes of data, and I'm
> > sure sure what you're doing here (sending a full page of data) works
> > for a SET_FEATURE command.
> > 
> > Do you have a way to test it (my NAND is not ONFI compliant)? By test
> > it, I mean, set a timing mode using SET_FEATURE and check if the new
> > mode has been applied using GET_FEATURE.  
> 
> I have an i.MX27 board with ONFI flash, but this one does not have the
> ONFI_OPT_CMD_SET_GET_FEATURES bit set, so I can't test it there.
> However, I can confirm that it works on an i.MX25. With the attached
> patch applied on vanilla v4.9-rc5 I get:
> 
> GET FEATURES. chip->onfi_timing_mode_default: 4
> timing before: 0x00
> timing after: 0x04

Okay, cool. I guess most NANDs are more tolerant than what's strictly
required in the ONFI spec.

Thanks for testing.

Boris

^ permalink raw reply

* [PATCH 1/2] kbuild: provide include/asm/asm-prototypes.h for ARM
From: Arnd Bergmann @ 2016-11-22 11:05 UTC (permalink / raw)
  To: linux-arm-kernel

This adds an asm/asm-prototypes.h header for ARM to fix the broken symbol
versioning for symbols exported from assembler files.

I couldn't find the correct prototypes for the compiler builtins,
so I went with the fake 'void f(void)' prototypes that we had
before, restoring the state before they were moved.

Originally I assumed that the problem was just a harmless warning
in unusual configurations, but as Uwe found, we actually need this
to load most modules when symbol versioning is enabled, as it is
in many distro kernels.

Cc: Uwe Kleine-K?nig <uwe@kleine-koenig.org>
Fixes: 4dd1837d7589 ("arm: move exports to definitions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Compared to the earlier version, I dropped the changes to the
csumpartial files, which now get handled correctly by Kbuild
even when the export comes from a macro, and I also dropped the
changes to the bitops files, which were already fixed in a
patch from Nico.

The patch applies cleanly on top of the rmk/fixes tree but has
no effect there, as it also needs 4efca4ed05cb ("kbuild: modversions
for EXPORT_SYMBOL() for asm") and cc6acc11cad1 ("kbuild: be more
careful about matching preprocessed asm ___EXPORT_SYMBOL").

With the combination of rmk/fixes, torvalds/master and these two
patches, symbol versioning works again on ARM. As it is still
broken on almost all other architectures (powerpc is fixed,
x86 has a patch), I wonder if we should make CONFIG_MODVERSIONS
as broken for everything else.
---
 arch/arm/include/asm/asm-prototypes.h | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 arch/arm/include/asm/asm-prototypes.h

diff --git a/arch/arm/include/asm/asm-prototypes.h b/arch/arm/include/asm/asm-prototypes.h
new file mode 100644
index 000000000000..04e5616a7b15
--- /dev/null
+++ b/arch/arm/include/asm/asm-prototypes.h
@@ -0,0 +1,34 @@
+#include <linux/arm-smccc.h>
+#include <linux/bitops.h>
+#include <linux/ftrace.h>
+#include <linux/io.h>
+#include <linux/platform_data/asoc-imx-ssi.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
+
+#include <asm/checksum.h>
+#include <asm/div64.h>
+#include <asm/memory.h>
+
+extern void __aeabi_idivmod(void);
+extern void __aeabi_idiv(void);
+extern void __aeabi_lasr(void);
+extern void __aeabi_llsl(void);
+extern void __aeabi_llsr(void);
+extern void __aeabi_lmul(void);
+extern void __aeabi_uidivmod(void);
+extern void __aeabi_uidiv(void);
+extern void __aeabi_ulcmp(void);
+
+extern void __ashldi3(void);
+extern void __ashrdi3(void);
+extern void __bswapdi2(void);
+extern void __bswapsi2(void);
+extern void __divsi3(void);
+extern void __do_div64(void);
+extern void __lshrdi3(void);
+extern void __modsi3(void);
+extern void __muldi3(void);
+extern void __ucmpdi2(void);
+extern void __udivsi3(void);
+extern void __umodsi3(void);
-- 
2.9.0

^ permalink raw reply related

* [PATCH 2/2] ARM: move mmiocpy/mmioset exports to io.c
From: Arnd Bergmann @ 2016-11-22 11:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161122110557.1533467-1-arnd@arndb.de>

The prototypes for mmioset/mmiocpy are intentionally hidden
inside of inline functions, which breaks the EXPORT_SYMBOL
statements when symbol versioning is enabled.

This moves the two exports from the files that implement the
code into the kernel/io.c file, adding another local declaration
there.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/kernel/io.c  | 7 +++++++
 arch/arm/lib/memcpy.S | 1 -
 arch/arm/lib/memset.S | 1 -
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/io.c b/arch/arm/kernel/io.c
index eedefe050022..c74746997626 100644
--- a/arch/arm/kernel/io.c
+++ b/arch/arm/kernel/io.c
@@ -82,3 +82,10 @@ void _memset_io(volatile void __iomem *dst, int c, size_t count)
 	}
 }
 EXPORT_SYMBOL(_memset_io);
+
+/* can't export them from memcpy.S/memset.S because of hidden declaration */
+void mmioset(void __iomem *addr, unsigned int c, size_t n);
+EXPORT_SYMBOL(mmioset);
+
+void mmiocpy(void *dest, const void __iomem *src, size_t n);
+EXPORT_SYMBOL(mmiocpy);
diff --git a/arch/arm/lib/memcpy.S b/arch/arm/lib/memcpy.S
index 1be5b6ddf37c..1f822fc52400 100644
--- a/arch/arm/lib/memcpy.S
+++ b/arch/arm/lib/memcpy.S
@@ -70,4 +70,3 @@ ENTRY(memcpy)
 ENDPROC(memcpy)
 ENDPROC(mmiocpy)
 EXPORT_SYMBOL(memcpy)
-EXPORT_SYMBOL(mmiocpy)
diff --git a/arch/arm/lib/memset.S b/arch/arm/lib/memset.S
index 7b72044cba62..6f075ca09abc 100644
--- a/arch/arm/lib/memset.S
+++ b/arch/arm/lib/memset.S
@@ -137,4 +137,3 @@ UNWIND( .fnend   )
 ENDPROC(memset)
 ENDPROC(mmioset)
 EXPORT_SYMBOL(memset)
-EXPORT_SYMBOL(mmioset)
-- 
2.9.0

^ 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