Refactor pinctrl driver for aobus / cbus

PATCH New Carlo Caione 6 participants 21 messages Mar 1, 2016 – Mar 18, 2016

Subsystems (3)

Device tree bindings Arm/Amlogic Meson SoC support Pin control

lore

Share & tools

lore

Carlo Caione

author

2016-03-01 22:04 UTC #0 lore raw

cover Refactor pinctrl driver for aobus / cbus

From: Carlo Caione <[email protected]>

The rationale behind this patchset is in this thread
http://www.spinics.net/lists/devicetree/msg115019.html

We basically want to introduce in the Amlogic Meson device-tree the CBUS and AOBUS nodes. To do this we need to split / refactor the pinctrl driver that is currently accessing at the same time registers on both these buses.

Carlo Caione (4):
pinctrl: amlogic: Separate some pin functions for Meson8 / Meson8b
pinctrl: amlogic: Make driver independent from two-domain
configuration
ARM: dts: amlogic: Split pinctrl device for Meson8 / Meson8b
documentation: Fix pinctrl documentation for Meson8 / Meson8b

 .../devicetree/bindings/pinctrl/meson,pinctrl.txt  |  36 +---
 arch/arm/boot/dts/meson8.dtsi                      |  57 ++++---
 arch/arm/boot/dts/meson8b.dtsi                     |  12 +-
 drivers/pinctrl/meson/pinctrl-meson.c              | 135 +++++++--------
 drivers/pinctrl/meson/pinctrl-meson.h              |  21 +--
 drivers/pinctrl/meson/pinctrl-meson8.c             | 137 +++++++++-------
 drivers/pinctrl/meson/pinctrl-meson8b.c            | 182 ++++++++++++---------
 7 files changed, 296 insertions(+), 284 deletions(-)

Signature · 1 line

1.9.1

Carlo Caione

author

2016-03-01 22:04 UTC #1 lore raw

PATCH 1/4 pinctrl: amlogic: Separate some pin functions for Meson8 / Meson8b

From: Carlo Caione <[email protected]>

Separate functions for pins controlled by different pin controllers.

Signed-off-by Carlo Caione <[email protected]>

---
 drivers/pinctrl/meson/pinctrl-meson8b.c | 35 +++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

drivers/pinctrl/meson/pinctrl-meson8b.c +27 -8

index 9677807..0c9572e 100644
@@ -694,7 +694,10 @@ static const char * const i2c_c_groups[] = {
 };
 
 static const char * const hdmi_groups[] = {
-	"hdmi_hpd", "hdmi_sda", "hdmi_scl", "hdmi_cec_0",
+	"hdmi_hpd", "hdmi_sda", "hdmi_scl", "hdmi_cec_0"
+};
+
+static const char * const hdmi_cec_groups[] = {
 	"hdmi_cec_1"
 };

@@ -770,12 +773,20 @@ static const char * const i2c_mst_ao_groups[] = {
 	"i2c_mst_sck_ao", "i2c_mst_sda_ao"
 };
 
-static const char * const clk_groups[] = {
-	"clk_24m_out", "clk_32k_in_out"
+static const char * const clk_24m_groups[] = {
+	"clk_24m_out",
+};
+
+static const char * const clk_32k_groups[] = {
+	"clk_32k_in_out",
 };
 
-static const char * const spdif_groups[] = {
-	"spdif_out_1", "spdif_out_0"
+static const char * const spdif_0_groups[] = {
+	"spdif_out_0"
+};
+
+static const char * const spdif_1_groups[] = {
+	"spdif_out_1"
 };
 
 static const char * const i2s_groups[] = {

@@ -789,7 +800,11 @@ static const char * const pwm_b_groups[] = {
 };
 
 static const char * const pwm_c_groups[] = {
-	"pwm_c0", "pwm_c1", "pwm_c2"
+	"pwm_c0", "pwm_c1"
+};
+
+static const char * const pwm_c_ao_groups[] = {
+	"pwm_c2"
 };
 
 static const char * const pwm_d_groups[] = {

@@ -827,6 +842,7 @@ static struct meson_pmx_func meson8b_functions[] = {
 	FUNCTION(uart_c),
 	FUNCTION(i2c_c),
 	FUNCTION(hdmi),
+	FUNCTION(hdmi_cec),
 	FUNCTION(spi),
 	FUNCTION(ethernet),
 	FUNCTION(i2c_a),

@@ -842,16 +858,19 @@ static struct meson_pmx_func meson8b_functions[] = {
 	FUNCTION(i2c_slave_ao),
 	FUNCTION(uart_ao_b),
 	FUNCTION(i2c_mst_ao),
-	FUNCTION(clk),
-	FUNCTION(spdif),
+	FUNCTION(clk_32k),
+	FUNCTION(spdif_0),
+	FUNCTION(spdif_1),
 	FUNCTION(i2s),
 	FUNCTION(pwm_b),
 	FUNCTION(pwm_c),
+	FUNCTION(pwm_c_ao),
 	FUNCTION(pwm_d),
 	FUNCTION(pwm_e),
 	FUNCTION(pwm_vs),
 	FUNCTION(tsin_a),
 	FUNCTION(tsin_b),
+	FUNCTION(clk_24m),
 };
 
 static struct meson_bank meson8b_banks[] = {

Signature · 1 line

1.9.1

Linus Walleij

2016-03-09 05:59 UTC #2 lore raw

PATCH 1/4 pinctrl: amlogic: Separate some pin functions for Meson8 / Meson8b

Quoted 5 lines · Carlo Caione

From: Carlo Caione <[email protected]>
Separate functions for pins controlled by different pin controllers.
Signed-off-by Carlo Caione <[email protected]>

Patch applied.

Yours,
Linus Walleij

Carlo Caione

author

2016-03-01 22:04 UTC #3 lore raw

PATCH 2/4 pinctrl: amlogic: Make driver independent from two-domain configuration

In reply to Carlo Caione

From: Carlo Caione <[email protected]>

In the Amlogic Meson8 / Meson8b platforms we have two different buses:
cbus and aobus, corresponding to 2 different power domains (regular and always-on). On each bus a different set of registers is mapped to manage muxes, GPIOs and in general to control a clear subset of the pins.

Considering this architecture, having two different pinctrl devices, one for each bus / power domain, makes much more sense than just having one single device.

Right now we have one single pin controller driver that uses two different domains (represented by 'gpio' and 'gpio-ao' sub-nodes in the DTS) to manage the set of registers on the two buses. This dual-domain configuration is hardcoded into the driver that strictly requires one domain for each bus in the same pin controller device.

With this patch we refactor the driver to allow splitting the driver in two parts. This change is needed to have a proper description of the HW in the device-tree where we want to introduce aobus and cbus.

Signed-off-by Carlo Caione <[email protected]>

---
 drivers/pinctrl/meson/pinctrl-meson.c   | 135 ++++++++++++---------------
 drivers/pinctrl/meson/pinctrl-meson.h   |  21 +----
 drivers/pinctrl/meson/pinctrl-meson8.c  | 137 +++++++++++++++------------
 drivers/pinctrl/meson/pinctrl-meson8b.c | 159 +++++++++++++++++---------------
 4 files changed, 228 insertions(+), 224 deletions(-)

drivers/pinctrl/meson/pinctrl-meson.c +59 ~~-76~~

index 50cab27..c50aaab 100644
@@ -104,15 +104,13 @@ static int meson_get_domain_and_bank(struct meson_pinctrl *pc, unsigned int pin,
 				     struct meson_bank **bank)
 {
 	struct meson_domain *d;
-	int i;
 
-	for (i = 0; i < pc->data->num_domains; i++) {
-		d = &pc->domains[i];
-		if (pin >= d->data->pin_base &&
-		    pin < d->data->pin_base + d->data->num_pins) {
-			*domain = d;
-			return meson_get_bank(d, pin, bank);
-		}
+	d = pc->domain;
+
+	if (pin >= d->data->pin_base &&
+	    pin < d->data->pin_base + d->data->num_pins) {
+		*domain = d;
+		return meson_get_bank(d, pin, bank);
 	}
 
 	return -EINVAL;

@@ -204,7 +202,7 @@ static void meson_pmx_disable_other_groups(struct meson_pinctrl *pc,
 		for (j = 0; j < group->num_pins; j++) {
 			if (group->pins[j] == pin) {
 				/* We have found a group using the pin */
-				domain = &pc->domains[group->domain];
+				domain = pc->domain;
 				regmap_update_bits(domain->reg_mux,
 						   group->reg * 4,
 						   BIT(group->bit), 0);

@@ -219,7 +217,7 @@ static int meson_pmx_set_mux(struct pinctrl_dev *pcdev, unsigned func_num,
 	struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev);
 	struct meson_pmx_func *func = &pc->data->funcs[func_num];
 	struct meson_pmx_group *group = &pc->data->groups[group_num];
-	struct meson_domain *domain = &pc->domains[group->domain];
+	struct meson_domain *domain = pc->domain;
 	int i, ret = 0;
 
 	dev_dbg(pc->dev, "enable function %s, group %s\n", func->name,

@@ -537,12 +535,20 @@ static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio)
 
 static const struct of_device_id meson_pinctrl_dt_match[] = {
 	{
-		.compatible = "amlogic,meson8-pinctrl",
-		.data = &meson8_pinctrl_data,
+		.compatible = "amlogic,meson8-cbus-pinctrl",
+		.data = &meson8_cbus_pinctrl_data,
+	},
+	{
+		.compatible = "amlogic,meson8b-cbus-pinctrl",
+		.data = &meson8b_cbus_pinctrl_data,
+	},
+	{
+		.compatible = "amlogic,meson8-aobus-pinctrl",
+		.data = &meson8_aobus_pinctrl_data,
 	},
 	{
-		.compatible = "amlogic,meson8b-pinctrl",
-		.data = &meson8b_pinctrl_data,
+		.compatible = "amlogic,meson8b-aobus-pinctrl",
+		.data = &meson8b_aobus_pinctrl_data,
 	},
 	{ },
 };

@@ -551,62 +557,46 @@ MODULE_DEVICE_TABLE(of, meson_pinctrl_dt_match);
 static int meson_gpiolib_register(struct meson_pinctrl *pc)
 {
 	struct meson_domain *domain;
-	int i, ret;
+	int ret;
 
-	for (i = 0; i < pc->data->num_domains; i++) {
-		domain = &pc->domains[i];
-
-		domain->chip.label = domain->data->name;
-		domain->chip.parent = pc->dev;
-		domain->chip.request = meson_gpio_request;
-		domain->chip.free = meson_gpio_free;
-		domain->chip.direction_input = meson_gpio_direction_input;
-		domain->chip.direction_output = meson_gpio_direction_output;
-		domain->chip.get = meson_gpio_get;
-		domain->chip.set = meson_gpio_set;
-		domain->chip.base = domain->data->pin_base;
-		domain->chip.ngpio = domain->data->num_pins;
-		domain->chip.can_sleep = false;
-		domain->chip.of_node = domain->of_node;
-		domain->chip.of_gpio_n_cells = 2;
-
-		ret = gpiochip_add_data(&domain->chip, domain);
-		if (ret) {
-			dev_err(pc->dev, "can't add gpio chip %s\n",
-				domain->data->name);
-			goto fail;
-		}
+	domain = pc->domain;
+
+	domain->chip.label = domain->data->name;
+	domain->chip.parent = pc->dev;
+	domain->chip.request = meson_gpio_request;
+	domain->chip.free = meson_gpio_free;
+	domain->chip.direction_input = meson_gpio_direction_input;
+	domain->chip.direction_output = meson_gpio_direction_output;
+	domain->chip.get = meson_gpio_get;
+	domain->chip.set = meson_gpio_set;
+	domain->chip.base = domain->data->pin_base;
+	domain->chip.ngpio = domain->data->num_pins;
+	domain->chip.can_sleep = false;
+	domain->chip.of_node = domain->of_node;
+	domain->chip.of_gpio_n_cells = 2;
+
+	ret = gpiochip_add_data(&domain->chip, domain);
+	if (ret) {
+		dev_err(pc->dev, "can't add gpio chip %s\n",
+			domain->data->name);
+		goto fail;
+	}
 
-		ret = gpiochip_add_pin_range(&domain->chip, dev_name(pc->dev),
-					     0, domain->data->pin_base,
-					     domain->chip.ngpio);
-		if (ret) {
-			dev_err(pc->dev, "can't add pin range\n");
-			goto fail;
-		}
+	ret = gpiochip_add_pin_range(&domain->chip, dev_name(pc->dev),
+				     0, domain->data->pin_base,
+				     domain->chip.ngpio);
+	if (ret) {
+		dev_err(pc->dev, "can't add pin range\n");
+		goto fail;
 	}
 
 	return 0;
 fail:
-	for (i--; i >= 0; i--)
-		gpiochip_remove(&pc->domains[i].chip);
+	gpiochip_remove(&pc->domain->chip);
 
 	return ret;
 }
 
-static struct meson_domain_data *meson_get_domain_data(struct meson_pinctrl *pc,
-						       struct device_node *np)
-{
-	int i;
-
-	for (i = 0; i < pc->data->num_domains; i++) {
-		if (!strcmp(np->name, pc->data->domain_data[i].name))
-			return &pc->data->domain_data[i];
-	}
-
-	return NULL;
-}
-
 static struct regmap_config meson_regmap_config = {
 	.reg_bits = 32,
 	.val_bits = 32,

@@ -643,7 +633,7 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
 {
 	struct device_node *np;
 	struct meson_domain *domain;
-	int i = 0, num_domains = 0;
+	int num_domains = 0;
 
 	for_each_child_of_node(node, np) {
 		if (!of_find_property(np, "gpio-controller", NULL))

@@ -651,29 +641,22 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
 		num_domains++;
 	}
 
-	if (num_domains != pc->data->num_domains) {
+	if (num_domains != 1) {
 		dev_err(pc->dev, "wrong number of subnodes\n");
 		return -EINVAL;
 	}
 
-	pc->domains = devm_kzalloc(pc->dev, num_domains *
-				   sizeof(struct meson_domain), GFP_KERNEL);
-	if (!pc->domains)
+	pc->domain = devm_kzalloc(pc->dev, sizeof(struct meson_domain), GFP_KERNEL);
+	if (!pc->domain)
 		return -ENOMEM;
 
+	domain = pc->domain;
+	domain->data = pc->data->domain_data;
+
 	for_each_child_of_node(node, np) {
 		if (!of_find_property(np, "gpio-controller", NULL))
 			continue;
 
-		domain = &pc->domains[i];
-
-		domain->data = meson_get_domain_data(pc, np);
-		if (!domain->data) {
-			dev_err(pc->dev, "domain data not found for node %s\n",
-				np->name);
-			return -ENODEV;
-		}
-
 		domain->of_node = np;
 
 		domain->reg_mux = meson_map_resource(pc, np, "mux");

@@ -699,7 +682,7 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
 			return PTR_ERR(domain->reg_gpio);
 		}
 
-		i++;
+		break;
 	}
 
 	return 0;

@@ -718,7 +701,7 @@ static int meson_pinctrl_probe(struct platform_device *pdev)
 
 	pc->dev = dev;
 	match = of_match_node(meson_pinctrl_dt_match, pdev->dev.of_node);
-	pc->data = (struct meson_pinctrl_data *)match->data;
+	pc->data = (struct meson_pinctrl_data *) match->data;
 
 	ret = meson_pinctrl_parse_dt(pc, pdev->dev.of_node);
 	if (ret)

drivers/pinctrl/meson/pinctrl-meson.h +5 ~~-16~~

index 0fe7d53..9c93e0d 100644
@@ -34,7 +34,6 @@ struct meson_pmx_group {
 	bool is_gpio;
 	unsigned int reg;
 	unsigned int bit;
-	unsigned int domain;
 };
 
 /**

@@ -144,7 +143,6 @@ struct meson_pinctrl_data {
 	unsigned int num_pins;
 	unsigned int num_groups;
 	unsigned int num_funcs;
-	unsigned int num_domains;
 };
 
 struct meson_pinctrl {

@@ -152,7 +150,7 @@ struct meson_pinctrl {
 	struct pinctrl_dev *pcdev;
 	struct pinctrl_desc desc;
 	struct meson_pinctrl_data *data;
-	struct meson_domain *domains;
+	struct meson_domain *domain;
 };
 
 #define PIN(x, b)	(b + x)

@@ -164,7 +162,6 @@ struct meson_pinctrl {
 		.num_pins = ARRAY_SIZE(grp ## _pins),			\
 		.reg = r,						\
 		.bit = b,						\
-		.domain = 0,						\
 	 }
 
 #define GPIO_GROUP(gpio, b)						\

@@ -175,16 +172,6 @@ struct meson_pinctrl {
 		.is_gpio = true,					\
 	 }
 
-#define GROUP_AO(grp, r, b)						\
-	{								\
-		.name = #grp,						\
-		.pins = grp ## _pins,					\
-		.num_pins = ARRAY_SIZE(grp ## _pins),			\
-		.reg = r,						\
-		.bit = b,						\
-		.domain = 1,						\
-	 }
-
 #define FUNCTION(fn)							\
 	{								\
 		.name = #fn,						\

@@ -208,5 +195,7 @@ struct meson_pinctrl {
 
 #define MESON_PIN(x, b) PINCTRL_PIN(PIN(x, b), #x)
 
-extern struct meson_pinctrl_data meson8_pinctrl_data;
-extern struct meson_pinctrl_data meson8b_pinctrl_data;
+extern struct meson_pinctrl_data meson8_cbus_pinctrl_data;
+extern struct meson_pinctrl_data meson8_aobus_pinctrl_data;
+extern struct meson_pinctrl_data meson8b_cbus_pinctrl_data;
+extern struct meson_pinctrl_data meson8b_aobus_pinctrl_data;

drivers/pinctrl/meson/pinctrl-meson8.c +77 ~~-60~~

200 lines not on this page · raw message

drivers/pinctrl/meson/pinctrl-meson8b.c +87 ~~-72~~

240 lines not on this page · raw message

Signature · 1 line

1.9.1

Linus Walleij

2016-03-09 06:01 UTC #4 lore raw

PATCH 2/4 pinctrl: amlogic: Make driver independent from two-domain configuration

Quoted 17 lines · Carlo Caione

From: Carlo Caione <[email protected]>
In the Amlogic Meson8 / Meson8b platforms we have two different buses:
cbus and aobus, corresponding to 2 different power domains (regular and always-on). On each bus a different set of registers is mapped to manage muxes, GPIOs and in general to control a clear subset of the pins.
Considering this architecture, having two different pinctrl devices, one for each bus / power domain, makes much more sense than just having one single device.
Right now we have one single pin controller driver that uses two different domains (represented by 'gpio' and 'gpio-ao' sub-nodes in the DTS) to manage the set of registers on the two buses. This dual-domain configuration is hardcoded into the driver that strictly requires one domain for each bus in the same pin controller device.

Quoted 5 lines

With this patch we refactor the driver to allow splitting the driver in two parts. This change is needed to have a proper description of the HW in the device-tree where we want to introduce aobus and cbus.
Signed-off-by Carlo Caione <[email protected]>

Patch applied.

Yours,
Linus Walleij

Kevin Hilman

maintainer

2016-03-17 03:31 UTC #5 lore raw

PATCH 2/4 pinctrl: amlogic: Make driver independent from two-domain configuration

In reply to Carlo Caione

Hi Carlo

Quoted 17 lines · Carlo Caione

From: Carlo Caione <[email protected]>
In the Amlogic Meson8 / Meson8b platforms we have two different buses:
cbus and aobus, corresponding to 2 different power domains (regular and always-on). On each bus a different set of registers is mapped to manage muxes, GPIOs and in general to control a clear subset of the pins.
Considering this architecture, having two different pinctrl devices, one for each bus / power domain, makes much more sense than just having one single device.
Right now we have one single pin controller driver that uses two different domains (represented by 'gpio' and 'gpio-ao' sub-nodes in the DTS) to manage the set of registers on the two buses. This dual-domain configuration is hardcoded into the driver that strictly requires one domain for each bus in the same pin controller device.

Quoted 5 lines

With this patch we refactor the driver to allow splitting the driver in two parts. This change is needed to have a proper description of the HW in the device-tree where we want to introduce aobus and cbus.
Signed-off-by Carlo Caione <[email protected]>

kernelci.org detected that the meson8b-odroidc1 started failing boot test in mainline[1] and I bisected it down to this patch, which is in mainline in he form of commit 9dab1868ec0d (pinctrl: amlogic: Make driver independent from two-domain configuration.)

I confirmed that reverting this patch on top of Linus' master branch (commit 9256d5a308c9) gets the odroid-c1 booting again.

Kevin

[1] https://kernelci.org/boot/meson8b-odroidc1/job/mainline/kernel/v4.5-1127-g9256d5a308c9/defconfig/multi_v7_defconfig/lab/lab-khilman/?_id=56e9af2359b5146a7a791516

Kevin Hilman

maintainer

2016-03-17 04:28 UTC #6 lore raw

PATCH 2/4 pinctrl: amlogic: Make driver independent from two-domain configuration

Quoted 26 lines · Kevin Hilman

Hi Carlo
Quoted 17 lines · Carlo Caione
From: Carlo Caione <[email protected]>
In the Amlogic Meson8 / Meson8b platforms we have two different buses:
cbus and aobus, corresponding to 2 different power domains (regular and always-on). On each bus a different set of registers is mapped to manage muxes, GPIOs and in general to control a clear subset of the pins.
Considering this architecture, having two different pinctrl devices, one for each bus / power domain, makes much more sense than just having one single device.
Right now we have one single pin controller driver that uses two different domains (represented by 'gpio' and 'gpio-ao' sub-nodes in the DTS) to manage the set of registers on the two buses. This dual-domain configuration is hardcoded into the driver that strictly requires one domain for each bus in the same pin controller device.
Quoted 5 lines
With this patch we refactor the driver to allow splitting the driver in two parts. This change is needed to have a proper description of the HW in the device-tree where we want to introduce aobus and cbus.
Signed-off-by Carlo Caione <[email protected]>

Quoted 7 lines

kernelci.org detected that the meson8b-odroidc1 started failing boot test in mainline[1] and I bisected it down to this patch, which is in mainline in he form of commit 9dab1868ec0d (pinctrl: amlogic: Make driver independent from two-domain configuration.)
I confirmed that reverting this patch on top of Linus' master branch (commit 9256d5a308c9) gets the odroid-c1 booting again.

Note that the board was failing to boot in next-20160316 as well[1], and reverting that same patch gets it booting again.

Kevin

[1] https://kernelci.org/boot/meson8b-odroidc1/job/next/kernel/next-20160316/defconfig/multi_v7_defconfig/lab/lab-khilman/?_id=56e991a359b514dd6b791517

Carlo Caione

author

2016-03-17 07:09 UTC #7 lore raw

PATCH 2/4 pinctrl: amlogic: Make driver independent from two-domain configuration

In reply to Kevin Hilman

Quoted 26 lines · Kevin Hilman

Hi Carlo
Quoted 17 lines · Carlo Caione
From: Carlo Caione <[email protected]>
In the Amlogic Meson8 / Meson8b platforms we have two different buses:
cbus and aobus, corresponding to 2 different power domains (regular and always-on). On each bus a different set of registers is mapped to manage muxes, GPIOs and in general to control a clear subset of the pins.
Considering this architecture, having two different pinctrl devices, one for each bus / power domain, makes much more sense than just having one single device.
Right now we have one single pin controller driver that uses two different domains (represented by 'gpio' and 'gpio-ao' sub-nodes in the DTS) to manage the set of registers on the two buses. This dual-domain configuration is hardcoded into the driver that strictly requires one domain for each bus in the same pin controller device.
Quoted 5 lines
With this patch we refactor the driver to allow splitting the driver in two parts. This change is needed to have a proper description of the HW in the device-tree where we want to introduce aobus and cbus.
Signed-off-by Carlo Caione <[email protected]>

Quoted 7 lines

kernelci.org detected that the meson8b-odroidc1 started failing boot test in mainline[1] and I bisected it down to this patch, which is in mainline in he form of commit 9dab1868ec0d (pinctrl: amlogic: Make driver independent from two-domain configuration.)
I confirmed that reverting this patch on top of Linus' master branch (commit 9256d5a308c9) gets the odroid-c1 booting again.

Yes, this is expected. Thank you for pointing this out.
The problem is that the driver changes ended up in mainline before I could pull the DTS changes (waiting for the Rob's ACK on documentation).
What's the fastest way to land the DTS changes in mainline at this point?

Thanks,

Signature · 1 line

Carlo Caione

Kevin Hilman

maintainer

2016-03-17 15:35 UTC #8 lore raw

PATCH 2/4 pinctrl: amlogic: Make driver independent from two-domain configuration

Quoted 32 lines · Carlo Caione

Quoted 31 lines · Kevin Hilman
Hi Carlo
Quoted 17 lines · Carlo Caione
From: Carlo Caione <[email protected]>
In the Amlogic Meson8 / Meson8b platforms we have two different buses:
cbus and aobus, corresponding to 2 different power domains (regular and always-on). On each bus a different set of registers is mapped to manage muxes, GPIOs and in general to control a clear subset of the pins.
Considering this architecture, having two different pinctrl devices, one for each bus / power domain, makes much more sense than just having one single device.
Right now we have one single pin controller driver that uses two different domains (represented by 'gpio' and 'gpio-ao' sub-nodes in the DTS) to manage the set of registers on the two buses. This dual-domain configuration is hardcoded into the driver that strictly requires one domain for each bus in the same pin controller device.
Quoted 5 lines
With this patch we refactor the driver to allow splitting the driver in two parts. This change is needed to have a proper description of the HW in the device-tree where we want to introduce aobus and cbus.
Signed-off-by Carlo Caione <[email protected]>
kernelci.org detected that the meson8b-odroidc1 started failing boot test in mainline[1] and I bisected it down to this patch, which is in mainline in he form of commit 9dab1868ec0d (pinctrl: amlogic: Make driver independent from two-domain configuration.)

Quoted 7 lines

Quoted 2 lines
I confirmed that reverting this patch on top of Linus' master branch (commit 9256d5a308c9) gets the odroid-c1 booting again.
Yes, this is expected. Thank you for pointing this out. The problem is that the driver changes ended up in mainline before I could pull the DTS changes (waiting for the Rob's ACK on documentation).

Do you have an ack now? or are you still waiting?

Quoted 1 line

What's the fastest way to land the DTS changes in mainline at this point?

When you've collected the acks, send a pull request through arm-soc so we can get it in via our fixes branch.

Kevin

Carlo Caione

author

2016-03-17 18:15 UTC #9 lore raw

PATCH 2/4 pinctrl: amlogic: Make driver independent from two-domain configuration

Quoted 1 line · Kevin Hilman

Do you have an ack now? or are you still waiting?

Still waiting for Rob.

Quoted 4 lines

Quoted 1 line
What's the fastest way to land the DTS changes in mainline at this point?
When you've collected the acks, send a pull request through arm-soc so we can get it in via our fixes branch.

Signature · 1 line

Carlo Caione

Kevin Hilman

maintainer

2016-03-18 16:14 UTC #10 lore raw

PATCH 2/4 pinctrl: amlogic: Make driver independent from two-domain configuration

Quoted 4 lines · Carlo Caione

Quoted 1 line · Kevin Hilman
Do you have an ack now? or are you still waiting?

Quoted 8 lines

Still waiting for Rob.
Quoted 4 lines
Quoted 1 line
What's the fastest way to land the DTS changes in mainline at this point?
When you've collected the acks, send a pull request through arm-soc so we can get it in via our fixes branch.
ok

If the Ack is not forthcoming ASAP, you should probably revert the change so that mainline/arm-soc/linux-next are not broken. For future reference, it's better to wait until all dependencies can be merged together.

Kevin

Carlo Caione

author

2016-03-01 22:04 UTC #11 lore raw

PATCH 3/4 ARM: dts: amlogic: Split pinctrl device for Meson8 / Meson8b

In reply to Carlo Caione

From: Carlo Caione <[email protected]>

Signed-off-by Carlo Caione <[email protected]>

---
 arch/arm/boot/dts/meson8.dtsi  | 57 ++++++++++++++++++++++++------------------
 arch/arm/boot/dts/meson8b.dtsi | 12 +++++++--
 2 files changed, 42 insertions(+), 27 deletions(-)

arch/arm/boot/dts/meson8.dtsi +32 ~~-25~~

index a2ddcb8..45619f6 100644
@@ -91,8 +91,8 @@
 		clock-frequency = <141666666>;
 	};
 
-	pinctrl: pinctrl at c1109880 {
-		compatible = "amlogic,meson8-pinctrl";
+	pinctrl_cbus: pinctrl at c1109880 {
+		compatible = "amlogic,meson8-cbus-pinctrl";
 		reg = <0xc1109880 0x10>;
 		#address-cells = <1>;
 		#size-cells = <1>;

@@ -108,29 +108,6 @@
 			#gpio-cells = <2>;
 		};
 
-		gpio_ao: ao-bank at c1108030 {
-			reg = <0xc8100014 0x4>,
-			      <0xc810002c 0x4>,
-			      <0xc8100024 0x8>;
-			reg-names = "mux", "pull", "gpio";
-			gpio-controller;
-			#gpio-cells = <2>;
-		};
-
-		uart_ao_a_pins: uart_ao_a {
-			mux {
-				groups = "uart_tx_ao_a", "uart_rx_ao_a";
-				function = "uart_ao";
-			};
-		};
-
-		i2c_ao_pins: i2c_mst_ao {
-			mux {
-				groups = "i2c_mst_sck_ao", "i2c_mst_sda_ao";
-				function = "i2c_mst_ao";
-			};
-		};
-
 		spi_nor_pins: nor {
 			mux {
 				groups = "nor_d", "nor_q", "nor_c", "nor_cs";

@@ -157,4 +134,34 @@
 		};
 	};
 
+	pinctrl_aobus: pinctrl at c8100084 {
+		compatible = "amlogic,meson8-aobus-pinctrl";
+		reg = <0xc8100084 0xc>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		gpio_ao: ao-bank at c1108030 {
+			reg = <0xc8100014 0x4>,
+			      <0xc810002c 0x4>,
+			      <0xc8100024 0x8>;
+			reg-names = "mux", "pull", "gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		uart_ao_a_pins: uart_ao_a {
+			mux {
+				groups = "uart_tx_ao_a", "uart_rx_ao_a";
+				function = "uart_ao";
+			};
+		};
+
+		i2c_ao_pins: i2c_mst_ao {
+			mux {
+				groups = "i2c_mst_sck_ao", "i2c_mst_sda_ao";
+				function = "i2c_mst_ao";
+			};
+		};
+	};
 }; /* end of / */

arch/arm/boot/dts/meson8b.dtsi +10 -2

index 8bad557..2bfe401 100644
@@ -155,8 +155,8 @@
 			reg = <0xc1108000 0x4>, <0xc1104000 0x460>;
 		};
 
-		pinctrl: pinctrl at c1109880 {
-			compatible = "amlogic,meson8b-pinctrl";
+		pinctrl_cbus: pinctrl at c1109880 {
+			compatible = "amlogic,meson8b-cbus-pinctrl";
 			reg = <0xc1109880 0x10>;
 			#address-cells = <1>;
 			#size-cells = <1>;

@@ -171,6 +171,14 @@
 				gpio-controller;
 				#gpio-cells = <2>;
 			};
+		};
+
+		pinctrl_aobus: pinctrl at c8100084 {
+			compatible = "amlogic,meson8b-aobus-pinctrl";
+			reg = <0xc8100084 0xc>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
 
 			gpio_ao: ao-bank at c1108030 {
 				reg = <0xc8100014 0x4>,

Signature · 1 line

1.9.1

Andreas Färber

2016-03-18 17:36 UTC #12 lore raw

PATCH 3/4 ARM: dts: amlogic: Split pinctrl device for Meson8 / Meson8b

Am 01.03.2016 um 23:04 schrieb Carlo Caione:

Quoted 7 lines

From: Carlo Caione <[email protected]>
Signed-off-by Carlo Caione <[email protected]>
---
 arch/arm/boot/dts/meson8.dtsi  | 57 ++++++++++++++++++++++++------------------
 arch/arm/boot/dts/meson8b.dtsi | 12 +++++++--
 2 files changed, 42 insertions(+), 27 deletions(-)

Looks sane to me,

Reviewed-by Andreas F?rber <[email protected]>

Andreas

Signature · 2 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton; HRB 21284 (AG N?rnberg)

Carlo Caione

author

2016-03-01 22:04 UTC #13 lore raw

PATCH 4/4 documentation: Fix pinctrl documentation for Meson8 / Meson8b

In reply to Carlo Caione

From: Carlo Caione <[email protected]>

Fix pin controller documentation with the new compatibles.

Signed-off-by Carlo Caione <[email protected]>

---
 .../devicetree/bindings/pinctrl/meson,pinctrl.txt  | 36 +++-------------------
 1 file changed, 5 insertions(+), 31 deletions(-)

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +5 ~~-31~~

index 3f6a524..51a8ebb 100644
@@ -1,13 +1,14 @@
 == Amlogic Meson pinmux controller ==
 
 Required properties for the root node:
- - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
+ - compatible: "amlogic,meson8-cbus-pinctrl", "amlogic,meson8b-cbus-pinctrl",
+   "amlogic,meson8-aobus-pinctrl" or "amlogic,meson8b-aobus-pinctrl"
  - reg: address and size of registers controlling irq functionality
 
 === GPIO sub-nodes ===
 
-The 2 power domains of the controller (regular and always-on) are
-represented as sub-nodes and each of them acts as a GPIO controller.
+Each power domain of the controller (regular and always-on) is
+represented as a sub-node and it acts as a GPIO controller.
 
 Required properties for sub-nodes are:
  - reg: should contain address and size for mux, pull-enable, pull and

@@ -18,10 +19,6 @@ Required properties for sub-nodes are:
  - gpio-controller: identifies the node as a gpio controller
  - #gpio-cells: must be 2
 
-Valid sub-node names are:
- - "banks" for the regular domain
- - "ao-bank" for the always-on domain
-
 === Other sub-nodes ===
 
 Child nodes without the "gpio-controller" represent some desired

@@ -45,7 +42,7 @@ pinctrl-bindings.txt
 === Example ===
 
 	pinctrl: pinctrl at c1109880 {
-		compatible = "amlogic,meson8-pinctrl";
+		compatible = "amlogic,meson8-cbus-pinctrl";
 		reg = <0xc1109880 0x10>;
 		#address-cells = <1>;
 		#size-cells = <1>;

@@ -61,15 +58,6 @@ pinctrl-bindings.txt
 			#gpio-cells = <2>;
                };
 
-		gpio_ao: ao-bank at c1108030 {
-			reg = <0xc8100014 0x4>,
-			      <0xc810002c 0x4>,
-			      <0xc8100024 0x8>;
-			reg-names = "mux", "pull", "gpio";
-			gpio-controller;
-			#gpio-cells = <2>;
-		};
-
 		nand {
 			mux {
 				groups = "nand_io", "nand_io_ce0", "nand_io_ce1",

@@ -79,18 +67,4 @@ pinctrl-bindings.txt
 				function = "nand";
 			};
 		};
-
-		uart_ao_a {
-			mux {
-				groups = "uart_tx_ao_a", "uart_rx_ao_a",
-					 "uart_cts_ao_a", "uart_rts_ao_a";
-				function = "uart_ao";
-			};
-
-			conf {
-				pins = "GPIOAO_0", "GPIOAO_1",
-				       "GPIOAO_2", "GPIOAO_3";
-				bias-disable;
-			};
-		};
 	};

Signature · 1 line

1.9.1

Rob Herring

maintainer

2016-03-05 04:26 UTC #14 lore raw

PATCH 4/4 documentation: Fix pinctrl documentation for Meson8 / Meson8b

Quoted 3 lines · Carlo Caione

From: Carlo Caione <[email protected]>
Fix pin controller documentation with the new compatibles.

What was wrong with it? (answer in the commit msg)

Quoted 11 lines

Signed-off-by Carlo Caione <[email protected]>
---
 .../devicetree/bindings/pinctrl/meson,pinctrl.txt  | 36 +++-------------------
 1 file changed, 5 insertions(+), 31 deletions(-)
Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +0 -0
index 3f6a524..51a8ebb 100644

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt line 5

Quoted 6 lines

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +2 -1

@@ -1,13 +1,14 @@
 == Amlogic Meson pinmux controller ==
 
 Required properties for the root node:
- - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
+ - compatible: "amlogic,meson8-cbus-pinctrl", "amlogic,meson8b-cbus-pinctrl",
+   "amlogic,meson8-aobus-pinctrl" or "amlogic,meson8b-aobus-pinctrl"

Reformat with 1 per line (assuming each are mutually exclusive).

Quoted 12 lines

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +2 -2

  - reg: address and size of registers controlling irq functionality
 
 === GPIO sub-nodes ===
 
-The 2 power domains of the controller (regular and always-on) are
-represented as sub-nodes and each of them acts as a GPIO controller.
+Each power domain of the controller (regular and always-on) is
+represented as a sub-node and it acts as a GPIO controller.
 
 Required properties for sub-nodes are:
  - reg: should contain address and size for mux, pull-enable, pull and

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt Required properties for sub-nodes are: · line 24

Quoted 7 lines

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +0 -4

@@ -18,10 +19,6 @@ Required properties for sub-nodes are:
  - gpio-controller: identifies the node as a gpio controller
  - #gpio-cells: must be 2
 
-Valid sub-node names are:
- - "banks" for the regular domain
- - "ao-bank" for the always-on domain
-

Why are these being removed?

Quoted 50 lines

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +1 ~~-24~~

 === Other sub-nodes ===
 
 Child nodes without the "gpio-controller" represent some desired

@@ -45,7 +42,7 @@ pinctrl-bindings.txt
 === Example ===
 
 	pinctrl: pinctrl at c1109880 {
-		compatible = "amlogic,meson8-pinctrl";
+		compatible = "amlogic,meson8-cbus-pinctrl";
 		reg = <0xc1109880 0x10>;
 		#address-cells = <1>;
 		#size-cells = <1>;

@@ -61,15 +58,6 @@ pinctrl-bindings.txt
 			#gpio-cells = <2>;
                };
 
-		gpio_ao: ao-bank at c1108030 {
-			reg = <0xc8100014 0x4>,
-			      <0xc810002c 0x4>,
-			      <0xc8100024 0x8>;
-			reg-names = "mux", "pull", "gpio";
-			gpio-controller;
-			#gpio-cells = <2>;
-		};
-
 		nand {
 			mux {
 				groups = "nand_io", "nand_io_ce0", "nand_io_ce1",

@@ -79,18 +67,4 @@ pinctrl-bindings.txt
 				function = "nand";
 			};
 		};
-
-		uart_ao_a {
-			mux {
-				groups = "uart_tx_ao_a", "uart_rx_ao_a",
-					 "uart_cts_ao_a", "uart_rts_ao_a";
-				function = "uart_ao";
-			};
-
-			conf {
-				pins = "GPIOAO_0", "GPIOAO_1",
-				       "GPIOAO_2", "GPIOAO_3";
-				bias-disable;
-			};
-		};
 	};

Signature · 1 line

1.9.1

Carlo Caione

author

2016-03-07 09:01 UTC #15 lore raw

PATCH 4/4 documentation: Fix pinctrl documentation for Meson8 / Meson8b

Quoted 6 lines · Rob Herring

Quoted 3 lines · Carlo Caione
From: Carlo Caione <[email protected]>
Fix pin controller documentation with the new compatibles.
What was wrong with it? (answer in the commit msg)

I'll extend the commit message.

[...]

Quoted 6 lines

Quoted 4 lines
Required properties for the root node:
- - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
+ - compatible: "amlogic,meson8-cbus-pinctrl", "amlogic,meson8b-cbus-pinctrl",
+ "amlogic,meson8-aobus-pinctrl" or "amlogic,meson8b-aobus-pinctrl"
Reformat with 1 per line (assuming each are mutually exclusive).

[...]

Quoted 6 lines

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt Required properties for sub-nodes are: · line 24
Quoted 4 lines
Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +0 -4
-Valid sub-node names are:
- - "banks" for the regular domain
- - "ao-bank" for the always-on domain
-
Why are these being removed?

Because we are not forced anymore to use those specific names for the sub-nodes.

Signature · 1 line

Carlo Caione

Carlo Caione

author

2016-03-09 09:41 UTC #16 lore raw

PATCH v2 4/4 documentation: Fix pinctrl documentation for Meson8 / Meson8b

In reply to Rob Herring

From: Carlo Caione <[email protected]>

Fix pin controller documentation introducing the new compatibles for the pinctrl drivers specific for aobus / cbus.

This is needed because we have changed the pin controller driver: we have now a single specialized pinctrl driver / compatible for each bus the controller is attached to, instead of one single driver dealing with all the controllers we have on different buses.

Signed-off-by Carlo Caione <[email protected]>

---
Hey Rob,
I'm resubmitting a V2 only for this patch since the driver changes have been already taken by Linus.

Thanks,

---
 .../devicetree/bindings/pinctrl/meson,pinctrl.txt  | 38 ++++------------------
 1 file changed, 7 insertions(+), 31 deletions(-)

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +7 ~~-31~~

index 3f6a524..ca6d283 100644
@@ -1,13 +1,16 @@
 == Amlogic Meson pinmux controller ==
 
 Required properties for the root node:
- - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
+ - compatible: one of "amlogic,meson8-cbus-pinctrl"
+		      "amlogic,meson8b-cbus-pinctrl"
+		      "amlogic,meson8-aobus-pinctrl"
+		      "amlogic,meson8b-aobus-pinctrl"
  - reg: address and size of registers controlling irq functionality
 
 === GPIO sub-nodes ===
 
-The 2 power domains of the controller (regular and always-on) are
-represented as sub-nodes and each of them acts as a GPIO controller.
+Each power domain of the controller (regular and always-on) is
+represented as a sub-node and it acts as a GPIO controller.
 
 Required properties for sub-nodes are:
  - reg: should contain address and size for mux, pull-enable, pull and

@@ -18,10 +21,6 @@ Required properties for sub-nodes are:
  - gpio-controller: identifies the node as a gpio controller
  - #gpio-cells: must be 2
 
-Valid sub-node names are:
- - "banks" for the regular domain
- - "ao-bank" for the always-on domain
-
 === Other sub-nodes ===
 
 Child nodes without the "gpio-controller" represent some desired

@@ -45,7 +44,7 @@ pinctrl-bindings.txt
 === Example ===
 
 	pinctrl: pinctrl at c1109880 {
-		compatible = "amlogic,meson8-pinctrl";
+		compatible = "amlogic,meson8-cbus-pinctrl";
 		reg = <0xc1109880 0x10>;
 		#address-cells = <1>;
 		#size-cells = <1>;

@@ -61,15 +60,6 @@ pinctrl-bindings.txt
 			#gpio-cells = <2>;
                };
 
-		gpio_ao: ao-bank at c1108030 {
-			reg = <0xc8100014 0x4>,
-			      <0xc810002c 0x4>,
-			      <0xc8100024 0x8>;
-			reg-names = "mux", "pull", "gpio";
-			gpio-controller;
-			#gpio-cells = <2>;
-		};
-
 		nand {
 			mux {
 				groups = "nand_io", "nand_io_ce0", "nand_io_ce1",

@@ -79,18 +69,4 @@ pinctrl-bindings.txt
 				function = "nand";
 			};
 		};
-
-		uart_ao_a {
-			mux {
-				groups = "uart_tx_ao_a", "uart_rx_ao_a",
-					 "uart_cts_ao_a", "uart_rts_ao_a";
-				function = "uart_ao";
-			};
-
-			conf {
-				pins = "GPIOAO_0", "GPIOAO_1",
-				       "GPIOAO_2", "GPIOAO_3";
-				bias-disable;
-			};
-		};
 	};

Signature · 1 line

2.5.0

Linus Walleij

2016-03-15 09:10 UTC #17 lore raw

PATCH v2 4/4 documentation: Fix pinctrl documentation for Meson8 / Meson8b

Quoted 15 lines · Carlo Caione

From: Carlo Caione <[email protected]>
Fix pin controller documentation introducing the new compatibles for the pinctrl drivers specific for aobus / cbus.
This is needed because we have changed the pin controller driver: we have now a single specialized pinctrl driver / compatible for each bus the controller is attached to, instead of one single driver dealing with all the controllers we have on different buses.
Signed-off-by Carlo Caione <[email protected]>
---
Hey Rob,
I'm resubmitting a V2 only for this patch since the driver changes have been already taken by Linus.

(...)

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt line 7

Quoted 6 lines

Required properties for the root node:
- - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
+ - compatible: one of "amlogic,meson8-cbus-pinctrl"
Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +3 -0
+                     "amlogic,meson8b-cbus-pinctrl"
+                     "amlogic,meson8-aobus-pinctrl"
+                     "amlogic,meson8b-aobus-pinctrl"

I guess I just have to merge this. It (IIUC) turns out that the driver is beta and prototypeish and the DT bindings were too early to be etched in stone, and any old device trees using it are not really working anyways.

We prefer that this not happen because we have a serious control issue when it comes to bindings, but sometimes it happens anyways. Like with divorces and other unfortunate stuff like that.

Rob are you OK with this change?

Yours,
Linus Walleij

Carlo Caione

2016-03-15 09:20 UTC #18 lore raw

PATCH v2 4/4 documentation: Fix pinctrl documentation for Meson8 / Meson8b

Quoted 25 lines · Linus Walleij

Quoted 15 lines · Carlo Caione
From: Carlo Caione <[email protected]>
Fix pin controller documentation introducing the new compatibles for the pinctrl drivers specific for aobus / cbus.
This is needed because we have changed the pin controller driver: we have now a single specialized pinctrl driver / compatible for each bus the controller is attached to, instead of one single driver dealing with all the controllers we have on different buses.
Signed-off-by Carlo Caione <[email protected]>
---
Hey Rob,
I'm resubmitting a V2 only for this patch since the driver changes have been already taken by Linus.
(...)
Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt line 7
Quoted 6 lines
Required properties for the root node:
- - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
+ - compatible: one of "amlogic,meson8-cbus-pinctrl"
Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +3 -0
+                     "amlogic,meson8b-cbus-pinctrl"
+                     "amlogic,meson8-aobus-pinctrl"
+                     "amlogic,meson8b-aobus-pinctrl"

Quoted 3 lines

I guess I just have to merge this. It (IIUC) turns out that the driver is beta and prototypeish and the DT bindings were too early to be etched in stone, and any old device trees using it are not really working anyways.

They are working but the platform support is so primitive (still trying to upstream the MMC driver) that really I wouldn't be too concerned of breaking the DT bindings at this stage of the development.
On the bright side we learned a lot from the Meson8b architecture so for mesongxbb things should be smoother.

Quoted 3 lines

We prefer that this not happen because we have a serious control issue when it comes to bindings, but sometimes it happens anyways. Like with divorces and other unfortunate stuff like that.

I know and agree with this, hopefully this will be a one time issue.

Thanks, I'll pick the DTS changes as soon as Rob ACKs this.

Signature · 1 line

Carlo Caione

Kevin Hilman

maintainer

2016-03-18 16:59 UTC #19 lore raw

PATCH v2 4/4 documentation: Fix pinctrl documentation for Meson8 / Meson8b

In reply to Linus Walleij

Quoted 29 lines · Linus Walleij

Quoted 15 lines · Carlo Caione
From: Carlo Caione <[email protected]>
Fix pin controller documentation introducing the new compatibles for the pinctrl drivers specific for aobus / cbus.
This is needed because we have changed the pin controller driver: we have now a single specialized pinctrl driver / compatible for each bus the controller is attached to, instead of one single driver dealing with all the controllers we have on different buses.
Signed-off-by Carlo Caione <[email protected]>
---
Hey Rob,
I'm resubmitting a V2 only for this patch since the driver changes have been already taken by Linus.
(...)
Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt line 7
Quoted 6 lines
Required properties for the root node:
- - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
+ - compatible: one of "amlogic,meson8-cbus-pinctrl"
Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +3 -0
+                     "amlogic,meson8b-cbus-pinctrl"
+                     "amlogic,meson8-aobus-pinctrl"
+                     "amlogic,meson8b-aobus-pinctrl"
I guess I just have to merge this. It (IIUC) turns out that the driver is beta and prototypeish and the DT bindings were too early to be etched in stone, and any old device trees using it are not really working anyways.

Quoted 5 lines

We prefer that this not happen because we have a serious control issue when it comes to bindings, but sometimes it happens anyways. Like with divorces and other unfortunate stuff like that.
Rob are you OK with this change?

Also, FYI, the driver change was merged without the corresponding DT, so this broke booting in linux-next, so either we merge the DT/bindings (preferred) or we revert the driver change until the DT/bindings are sorted out.

Kevin

Andreas Färber

2016-03-18 17:45 UTC #20 lore raw

PATCH v2 4/4 documentation: Fix pinctrl documentation for Meson8 / Meson8b

In reply to Carlo Caione

Am 09.03.2016 um 10:41 schrieb Carlo Caione:

Quoted 32 lines

From: Carlo Caione <[email protected]>
Fix pin controller documentation introducing the new compatibles for the pinctrl drivers specific for aobus / cbus.
This is needed because we have changed the pin controller driver: we have now a single specialized pinctrl driver / compatible for each bus the controller is attached to, instead of one single driver dealing with all the controllers we have on different buses.
Signed-off-by Carlo Caione <[email protected]>
---
Hey Rob,
I'm resubmitting a V2 only for this patch since the driver changes have been already taken by Linus.
Thanks,
---
 .../devicetree/bindings/pinctrl/meson,pinctrl.txt  | 38 ++++------------------
 1 file changed, 7 insertions(+), 31 deletions(-)
Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +2 -1
index 3f6a524..ca6d283 100644
@@ -1,13 +1,16 @@
 == Amlogic Meson pinmux controller ==
 
 Required properties for the root node:
- - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
+ - compatible: one of "amlogic,meson8-cbus-pinctrl"
+		      "amlogic,meson8b-cbus-pinctrl"

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt line 13

Quoted 10 lines

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +4 -2

+		      "amlogic,meson8-aobus-pinctrl"
+		      "amlogic,meson8b-aobus-pinctrl"
  - reg: address and size of registers controlling irq functionality
 
 === GPIO sub-nodes ===
 
-The 2 power domains of the controller (regular and always-on) are
-represented as sub-nodes and each of them acts as a GPIO controller.
+Each power domain of the controller (regular and always-on) is
+represented as a sub-node and it acts as a GPIO controller.

This paragraph still sounds outdated to me: With regular and always-on being separate devices now, there is no "and" here, nor different power domains as "GPIO sub-nodes" (heading).

Otherwise looks okay to me, and +1 for doing the incompatible split. gxbb already models cbus and aobus as simple-bus'es.

Regards,
Andreas

Quoted 59 lines

Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt +1 ~~-28~~

 Required properties for sub-nodes are:
  - reg: should contain address and size for mux, pull-enable, pull and

@@ -18,10 +21,6 @@ Required properties for sub-nodes are:
  - gpio-controller: identifies the node as a gpio controller
  - #gpio-cells: must be 2
 
-Valid sub-node names are:
- - "banks" for the regular domain
- - "ao-bank" for the always-on domain
-
 === Other sub-nodes ===
 
 Child nodes without the "gpio-controller" represent some desired

@@ -45,7 +44,7 @@ pinctrl-bindings.txt
 === Example ===
 
 	pinctrl: pinctrl at c1109880 {
-		compatible = "amlogic,meson8-pinctrl";
+		compatible = "amlogic,meson8-cbus-pinctrl";
 		reg = <0xc1109880 0x10>;
 		#address-cells = <1>;
 		#size-cells = <1>;

@@ -61,15 +60,6 @@ pinctrl-bindings.txt
 			#gpio-cells = <2>;
                };
 
-		gpio_ao: ao-bank at c1108030 {
-			reg = <0xc8100014 0x4>,
-			      <0xc810002c 0x4>,
-			      <0xc8100024 0x8>;
-			reg-names = "mux", "pull", "gpio";
-			gpio-controller;
-			#gpio-cells = <2>;
-		};
-
 		nand {
 			mux {
 				groups = "nand_io", "nand_io_ce0", "nand_io_ce1",

@@ -79,18 +69,4 @@ pinctrl-bindings.txt
 				function = "nand";
 			};
 		};
-
-		uart_ao_a {
-			mux {
-				groups = "uart_tx_ao_a", "uart_rx_ao_a",
-					 "uart_cts_ao_a", "uart_rts_ao_a";
-				function = "uart_ao";
-			};
-
-			conf {
-				pins = "GPIOAO_0", "GPIOAO_1",
-				       "GPIOAO_2", "GPIOAO_3";
-				bias-disable;
-			};
-		};
 	};

Signature · 2 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton; HRB 21284 (AG N?rnberg)