WTF: patch "[PATCH] mmc: dw_mmc: Use device_property_read instead of" was seriously submitted to be applied to the 4.12-stable tree?

PATCH [email protected] 2 participants 11 messages Jul 24, 2017 – Aug 8, 2017

Subsystems (3)

Stable branch MMC, SD and SDIO Synopsys DesignWare MMC/SD/SDIO driver

lore

Share & tools

lore

[email protected]

author maintainer

2017-07-24 23:32 UTC #0 lore raw

The patch below was submitted to be applied to the 4.12-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable_kernel_rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From 852ff5fea9eb6a9799f1881d6df2cd69a9e6eed5 Mon Sep 17 00:00:00 2001

From: David Woods <[email protected]>
Date: Fri, 26 May 2017 17:53:20 -0400
Subject: [PATCH] mmc: dw_mmc: Use device_property_read instead of
of_property_read

Using the device_property interfaces allows the dw_mmc driver to work on platforms which run on either device tree or ACPI.

Signed-off-by David Woods <[email protected]>

Reviewed-by Chris Metcalf <[email protected]>

Cc [email protected]

Acked-by Jaehoon Chung <[email protected]>

Signed-off-by Ulf Hansson <[email protected]>

drivers/mmc/host/dw_mmc.c +12 ~~-12~~

index 0e2d6f7de469..4a0841cee39b 100644
@@ -2723,8 +2723,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	host->slot[id] = slot;
 
 	mmc->ops = &dw_mci_ops;
-	if (of_property_read_u32_array(host->dev->of_node,
-				       "clock-freq-min-max", freq, 2)) {
+	if (device_property_read_u32_array(host->dev, "clock-freq-min-max",
+					   freq, 2)) {
 		mmc->f_min = DW_MCI_FREQ_MIN;
 		mmc->f_max = DW_MCI_FREQ_MAX;
 	} else {

@@ -2828,7 +2828,6 @@ static void dw_mci_init_dma(struct dw_mci *host)
 {
 	int addr_config;
 	struct device *dev = host->dev;
-	struct device_node *np = dev->of_node;
 
 	/*
 	* Check tansfer mode from HCON[17:16]

@@ -2889,8 +2888,9 @@ static void dw_mci_init_dma(struct dw_mci *host)
 		dev_info(host->dev, "Using internal DMA controller.\n");
 	} else {
 		/* TRANS_MODE_EDMAC: check dma bindings again */
-		if ((of_property_count_strings(np, "dma-names") < 0) ||
-		    (!of_find_property(np, "dmas", NULL))) {
+		if ((device_property_read_string_array(dev, "dma-names",
+						       NULL, 0) < 0) ||
+		    !device_property_present(dev, "dmas")) {
 			goto no_dma;
 		}
 		host->dma_ops = &dw_mci_edmac_ops;

@@ -2957,7 +2957,6 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 {
 	struct dw_mci_board *pdata;
 	struct device *dev = host->dev;
-	struct device_node *np = dev->of_node;
 	const struct dw_mci_drv_data *drv_data = host->drv_data;
 	int ret;
 	u32 clock_frequency;

@@ -2974,20 +2973,21 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 	}
 
 	/* find out number of slots supported */
-	of_property_read_u32(np, "num-slots", &pdata->num_slots);
+	device_property_read_u32(dev, "num-slots", &pdata->num_slots);
 
-	if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth))
+	if (device_property_read_u32(dev, "fifo-depth", &pdata->fifo_depth))
 		dev_info(dev,
 			 "fifo-depth property not found, using value of FIFOTH register as default\n");
 
-	of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
+	device_property_read_u32(dev, "card-detect-delay",
+				 &pdata->detect_delay_ms);
 
-	of_property_read_u32(np, "data-addr", &host->data_addr_override);
+	device_property_read_u32(dev, "data-addr", &host->data_addr_override);
 
-	if (of_get_property(np, "fifo-watermark-aligned", NULL))
+	if (device_property_present(dev, "fifo-watermark-aligned"))
 		host->wm_aligned = true;
 
-	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
+	if (!device_property_read_u32(dev, "clock-frequency", &clock_frequency))
 		pdata->bus_hz = clock_frequency;
 
 	if (drv_data && drv_data->parse_dt) {

Chris Metcalf

2017-07-25 01:23 UTC #1 lore raw

Quoted 7 lines · On 7/24/2017 7:32 PM, [email protected] wrote:

The patch below was submitted to be applied to the 4.12-stable tree.
I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable_kernel_rules.rst.
[...]

Quoted 8 lines

Using the device_property interfaces allows the dw_mmc driver to work on platforms which run on either device tree or ACPI.
Signed-off-by David Woods <[email protected]>
Reviewed-by Chris Metcalf <[email protected]>
Cc [email protected]
Acked-by Jaehoon Chung <[email protected]>
Signed-off-by Ulf Hansson <[email protected]>

I encouraged Dave to cc stable, so this is on me if it was a bad call.

This bug is the only thing blocking our ARM64 hardware (Mellanox BlueField, google if curious) from booting up with RedHat 7 on our standard hardware config with an eMMC rootfs. RedHat 7 requires ACPI for server configuration builds, and the eMMC drivers didn't support ACPI, just device tree, so the chip couldn't boot.

The two changes are obviously correct (mechanical substitution of "of_*" to "device_*"), short, and fix a clear bug in booting up RHEL. Now obviously we are working with RedHat directly to get them to include the fix (RH bugzilla 1466917) but it seemed plausible to push the fix upstream so it would get picked up elsewhere as well and enable booting older mainstream ARM64 kernels on our hardware.

Thanks!

Signature · 2 lines

Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

Greg KH

author maintainer

2017-07-25 14:48 UTC #2 lore raw

Quoted 25 lines · Chris Metcalf

Quoted 7 lines · On 7/24/2017 7:32 PM, [email protected] wrote:
The patch below was submitted to be applied to the 4.12-stable tree.
I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable_kernel_rules.rst.
[...]
Quoted 8 lines
Using the device_property interfaces allows the dw_mmc driver to work on platforms which run on either device tree or ACPI.
Signed-off-by David Woods <[email protected]>
Reviewed-by Chris Metcalf <[email protected]>
Cc [email protected]
Acked-by Jaehoon Chung <[email protected]>
Signed-off-by Ulf Hansson <[email protected]>
I encouraged Dave to cc stable, so this is on me if it was a bad call.
This bug is the only thing blocking our ARM64 hardware (Mellanox BlueField, google if curious) from booting up with RedHat 7 on our standard hardware config with an eMMC rootfs. RedHat 7 requires ACPI for server configuration builds, and the eMMC drivers didn't support ACPI, just device tree, so the chip couldn't boot.

Quoted 6 lines

The two changes are obviously correct (mechanical substitution of "of_*" to "device_*"), short, and fix a clear bug in booting up RHEL. Now obviously we are working with RedHat directly to get them to include the fix (RH bugzilla 1466917) but it seemed plausible to push the fix upstream so it would get picked up elsewhere as well and enable booting older mainstream ARM64 kernels on our hardware.

So is this the _only_ change that is needed to get a 4.9 and/or a 4.12 kernel running on your hardware? Or would they need other ones as well?

thanks,

greg k-h

Chris Metcalf

2017-07-25 15:55 UTC #3 lore raw

Quoted 25 lines · Greg KH

Quoted 24 lines · Chris Metcalf
Quoted 7 lines · On 7/24/2017 7:32 PM, [email protected] wrote:
The patch below was submitted to be applied to the 4.12-stable tree.
I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable_kernel_rules.rst.
[...]
Quoted 8 lines
Using the device_property interfaces allows the dw_mmc driver to work on platforms which run on either device tree or ACPI.
Signed-off-by David Woods <[email protected]>
Reviewed-by Chris Metcalf <[email protected]>
Cc [email protected]
Acked-by Jaehoon Chung <[email protected]>
Signed-off-by Ulf Hansson <[email protected]>
I encouraged Dave to cc stable, so this is on me if it was a bad call.
This bug is the only thing blocking our ARM64 hardware (Mellanox BlueField, google if curious) from booting up with RedHat 7 on our standard hardware config with an eMMC rootfs. RedHat 7 requires ACPI for server configuration builds, and the eMMC drivers didn't support ACPI, just device tree, so the chip couldn't boot.

Quoted 8 lines

Quoted 6 lines
The two changes are obviously correct (mechanical substitution of "of_*" to "device_*"), short, and fix a clear bug in booting up RHEL. Now obviously we are working with RedHat directly to get them to include the fix (RH bugzilla 1466917) but it seemed plausible to push the fix upstream so it would get picked up elsewhere as well and enable booting older mainstream ARM64 kernels on our hardware.
So is this the _only_ change that is needed to get a 4.9 and/or a 4.12 kernel running on your hardware? Or would they need other ones as well?

As far as we know, these are the only patches needed to get RHEL booted. There are additional drivers for new IP on our SoC, but those are orthogonal to actually booting up the system.

For the 4.9 backport you may want to also just take e44bb0cbdc88 ("device property: Make dev_fwnode() public") or else just open-code the place in the patch where dev_fwnode() is used.

Signature · 2 lines

Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

Chris Metcalf

2017-07-28 13:53 UTC #4 lore raw

Ping? Greg, I see that you released 4.12.4/4.9.40 without these patches, but not having heard back from you, I wondered if my reply below was lost and we were both waiting on each other. Thanks!

Quoted 43 lines · Chris Metcalf

Quoted 25 lines · Greg KH
Quoted 24 lines · Chris Metcalf
Quoted 7 lines · On 7/24/2017 7:32 PM, [email protected] wrote:
The patch below was submitted to be applied to the 4.12-stable tree.
I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable_kernel_rules.rst.
[...]
Quoted 8 lines
Using the device_property interfaces allows the dw_mmc driver to work on platforms which run on either device tree or ACPI.
Signed-off-by David Woods <[email protected]>
Reviewed-by Chris Metcalf <[email protected]>
Cc [email protected]
Acked-by Jaehoon Chung <[email protected]>
Signed-off-by Ulf Hansson <[email protected]>
I encouraged Dave to cc stable, so this is on me if it was a bad call.
This bug is the only thing blocking our ARM64 hardware (Mellanox BlueField, google if curious) from booting up with RedHat 7 on our standard hardware config with an eMMC rootfs. RedHat 7 requires ACPI for server configuration builds, and the eMMC drivers didn't support ACPI, just device tree, so the chip couldn't boot.
Quoted 8 lines
Quoted 6 lines
The two changes are obviously correct (mechanical substitution of "of_*" to "device_*"), short, and fix a clear bug in booting up RHEL. Now obviously we are working with RedHat directly to get them to include the fix (RH bugzilla 1466917) but it seemed plausible to push the fix upstream so it would get picked up elsewhere as well and enable booting older mainstream ARM64 kernels on our hardware.
So is this the _only_ change that is needed to get a 4.9 and/or a 4.12 kernel running on your hardware? Or would they need other ones as well?
As far as we know, these are the only patches needed to get RHEL booted. There are additional drivers for new IP on our SoC, but those are orthogonal to actually booting up the system.
For the 4.9 backport you may want to also just take e44bb0cbdc88 ("device property: Make dev_fwnode() public") or else just open-code the place in the patch where dev_fwnode() is used.

Signature · 2 lines

Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

Greg KH

author maintainer

2017-07-28 23:48 UTC #5 lore raw

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

Quoted 4 lines · Chris Metcalf

Ping? Greg, I see that you released 4.12.4/4.9.40 without these patches, but
not having heard back from you, I wondered if my reply below was lost and we were both waiting on each other. Thanks!

No, it just went to the bottom of more important patches (i.e. bugfixes.) I'll reconsider this at a later point in time when I get a chance...

thanks,

greg k-h

Chris Metcalf

2017-07-29 02:03 UTC #6 lore raw

Quoted 1 line · Greg KH

https://daringfireball.net/2007/07/on_top

Good for you for trying to educate - hopefully it will stick, occasionally.

In 30+ years of emailing, I can honestly say that "Ping" emails are the only ones where I appreciate top-posting. The whole point of such an email is a "TL;DR" sanity check, but at the same time, quoting the entire email is actually important when you think the previous email might have been lost in transit or misplaced. I get irritated when someone quotes the whole damn email and I get to the bottom just to find a "ping".

Quoted 6 lines

Quoted 3 lines
Ping? Greg, I see that you released 4.12.4/4.9.40 without these patches, but not having heard back from you, I wondered if my reply below was lost and we were both waiting on each other. Thanks!
No, it just went to the bottom of more important patches (i.e. bugfixes.) I'll reconsider this at a later point in time when I get a chance...

Great, thanks!

Signature · 2 lines

Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

Greg KH

author maintainer

2017-08-07 19:51 UTC #7 lore raw

Quoted 9 lines · Chris Metcalf

Quoted 6 lines · Greg KH
Quoted 3 lines
Ping? Greg, I see that you released 4.12.4/4.9.40 without these patches, but not having heard back from you, I wondered if my reply below was lost and we were both waiting on each other. Thanks!
No, it just went to the bottom of more important patches (i.e. bugfixes.) I'll reconsider this at a later point in time when I get a chance...
Great, thanks!

Ok, I've applied this for 4.12 and 4.9 trees, let's see what breaks...

Greg KH

author maintainer

2017-08-07 21:01 UTC #8 lore raw

Quoted 5 lines · Greg KH

Quoted 4 lines · Chris Metcalf
Quoted 3 lines · Greg KH
Quoted 3 lines
Ping? Greg, I see that you released 4.12.4/4.9.40 without these patches, but not having heard back from you, I wondered if my reply below was lost and we were both waiting on each other. Thanks!

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
No, it just went to the bottom of more important patches (i.e. bugfixes.) I'll reconsider this at a later point in time when I get a chance...
Great, thanks!
Ok, I've applied this for 4.12 and 4.9 trees, let's see what breaks...

Ok, 4.9 breaks badly, can you provide working backports of the two patches for that kernel tree please?

thanks,

greg k-h

Chris Metcalf

2017-08-08 14:29 UTC #9 lore raw

Quoted 4 lines · Greg KH

Quoted 1 line · Greg KH
Ok, I've applied this for 4.12 and 4.9 trees, let's see what breaks...
Ok, 4.9 breaks badly, can you provide working backports of the two patches for that kernel tree please?

Sure. There were some additional of_property_*() calls added since 4.9, so the patches failed to apply cleanly. I'll reply to the two "FAILED" emails with properly backported versions.

Note that you will have to apply commit e44bb0cbdc88 ("device
property: Make dev_fwnode() public") as well to allow the code to compile, since it uses dev_fwnode.

Signature · 2 lines

Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

Greg KH

author maintainer

2017-08-08 14:51 UTC #10 lore raw

Quoted 7 lines · Chris Metcalf

Quoted 5 lines · Greg KH
Quoted 2 lines · Greg KH
Ok, I've applied this for 4.12 and 4.9 trees, let's see what breaks...
Ok, 4.9 breaks badly, can you provide working backports of the two patches for that kernel tree please?

Quoted 7 lines

Sure. There were some additional of_property_*() calls added since 4.9, so the patches failed to apply cleanly. I'll reply to the two "FAILED" emails with properly backported versions.
Note that you will have to apply commit e44bb0cbdc88 ("device
property: Make dev_fwnode() public") as well to allow the code to compile, since it uses dev_fwnode.

Yes, that patch is in the tree now.

thanks,

greg k-h