Re: [PATCH v1 1/9] i2c: designware: Move has_acpi_companion() to common code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/31/23 23:14, Andy Shevchenko wrote:
On Fri, Jul 28, 2023 at 02:33:07PM +0300, Jarkko Nikula wrote:
On 7/26/23 00:45, Andi Shyti wrote:
On Tue, Jul 25, 2023 at 05:30:15PM +0300, Andy Shevchenko wrote:

...

-int i2c_dw_acpi_configure(struct device *device)
+static void i2c_dw_acpi_do_configure(struct dw_i2c_dev *dev, struct device *device)

Because of this dual dev pointer obscurity which is cleaned in the next
patch and Andi's comment below in my opinion it makes sense to combine
patches 1 and 2.

Besides that these 2 are logically slightly different, the changes don't drop
the duality here. And there is also the other patch at the end of the series
that makes the below disappear.

Not sure that any of these would be the best approach (Git commit is cheap,
maintenance and backporting might be harder). So, ideas are welcome!

Unless I'm missing something you won't need to carry both struct dw_i2c_dev *dev and struct device *device since struct dw_i2c_dev carries it already and it's set before calling the dw_i2c_of_configure() and i2c_dw_acpi_configure().

Also it feels needless to add new _do_configure() functions since only reason for them seems to be how patches are organized now.

So if instead of this in i2c_dw_fw_parse_and_configure()

	if (is_of_node(fwnode))
		i2c_dw_of_do_configure(dev, dev->dev);
	else if (is_acpi_node(fwnode))
		i2c_dw_acpi_do_configure(dev, dev->dev);

let end result be

	if (is_of_node(fwnode))
		i2c_dw_of_configure(dev);
	else if (is_acpi_node(fwnode))
		i2c_dw_acpi_configure(dev);

My gut feeling says patchset would be a bit simpler if we aim for this end result in mind.

Simplest patches like int to void return type conversion first since either i2c_dw_acpi_configure() and dw_i2c_of_configure() return is not used now. Then perhaps dw_i2c_of_configure() renaming.

Moving to common code I don't know how well it's splittable into smaller patches or would single bigger patch look better.



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux