23.11.2021 10:15, Akhil R пишет:

Add support for ACPI based device registration so that the driver can be also enabled through ACPI table.

Signed-off-by: Akhil R akhilrajeev@nvidia.com

drivers/i2c/busses/i2c-tegra.c | 52 ++++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index c883044..8e47889 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -6,6 +6,7 @@

• Author: Colin Cross ccross@android.com

*/ +#include <linux/acpi.h> #include <linux/bitfield.h> #include <linux/clk.h> #include <linux/delay.h> @@ -608,6 +609,7 @@ static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev) static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) { u32 val, clk_divisor, clk_multiplier, tsu_thd, tlow, thigh, non_hs_mode;

• acpi_handle handle = ACPI_HANDLE(i2c_dev->dev); int err;

/* @@ -618,7 +620,11 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) * emit a noisy warning on error, which won't stay unnoticed and * won't hose machine entirely. */

• err = reset_control_reset(i2c_dev->rst);

• if (handle && acpi_has_method(handle, "_RST"))

Which SoC version doesn't have "_RST" method? If neither, then please remove this check.

• err = (acpi_evaluate_object(handle, "_RST", NULL, NULL));
  

Please remove parens around acpi_evaluate_object(). Why you added them?

• else

• err = reset_control_reset(i2c_dev->rst);
  

• WARN_ON_ONCE(err);

if (i2c_dev->is_dvc) @@ -1627,12 +1633,12 @@ static void tegra_i2c_parse_dt(struct tegra_i2c_dev *i2c_dev) bool multi_mode; int err;

• err = of_property_read_u32(np, "clock-frequency",

• 		   &i2c_dev->bus_clk_rate);
  

• err = device_property_read_u32(i2c_dev->dev, "clock-frequency",

• 		       &i2c_dev->bus_clk_rate);
  

  if (err) i2c_dev->bus_clk_rate = I2C_MAX_STANDARD_MODE_FREQ;

• multi_mode = of_property_read_bool(np, "multi-master");

• multi_mode = device_property_read_bool(i2c_dev->dev, "multi-master"); i2c_dev->multimaster_mode = multi_mode;

if (of_device_is_compatible(np, "nvidia,tegra20-i2c-dvc")) @@ -1642,10 +1648,25 @@ static void tegra_i2c_parse_dt(struct tegra_i2c_dev *i2c_dev) i2c_dev->is_vi = true; }

How are you going to differentiate the VI I2C from a non-VI? This doesn't look right.

+static int tegra_i2c_init_reset(struct tegra_i2c_dev *i2c_dev) +{

• if (has_acpi_companion(i2c_dev->dev))

• return 0;
  

• i2c_dev->rst = devm_reset_control_get_exclusive(i2c_dev->dev, "i2c");
• if (IS_ERR(i2c_dev->rst))

• return PTR_ERR(i2c_dev->rst);
  

• return 0;

+}

static int tegra_i2c_init_clocks(struct tegra_i2c_dev *i2c_dev) { int err;

• if (has_acpi_companion(i2c_dev->dev))

• return 0;
  

• i2c_dev->clocks[i2c_dev->nclocks++].id = "div-clk";

if (i2c_dev->hw == &tegra20_i2c_hw || i2c_dev->hw == &tegra30_i2c_hw) @@ -1720,7 +1741,7 @@ static int tegra_i2c_probe(struct platform_device *pdev) init_completion(&i2c_dev->msg_complete); init_completion(&i2c_dev->dma_complete);

• i2c_dev->hw = of_device_get_match_data(&pdev->dev);

• i2c_dev->hw = device_get_match_data(&pdev->dev); i2c_dev->cont_id = pdev->id; i2c_dev->dev = &pdev->dev;

@@ -1746,15 +1767,13 @@ static int tegra_i2c_probe(struct platform_device *pdev) if (err) return err;

• i2c_dev->rst = devm_reset_control_get_exclusive(i2c_dev->dev, "i2c");
• if (IS_ERR(i2c_dev->rst)) {

• dev_err_probe(i2c_dev->dev, PTR_ERR(i2c_dev->rst),
  

• 	      "failed to get reset control\n");
  

• return PTR_ERR(i2c_dev->rst);
  

• }
• tegra_i2c_parse_dt(i2c_dev);

• err = tegra_i2c_init_reset(i2c_dev);
• if (err)

• return dev_err_probe(i2c_dev->dev, err,
  

• 		      "failed to get reset control\n");
  

This is inconsistent with tegra_i2c_init_clocks() which returns err directly and prints error message within the function. Please move the dev_err_probe() into tegra_i2c_init_reset() to make it consistent, like I suggested before.

Please don't reply with a new version of the patch to the old thread, always send a new version separately. Otherwise it's more difficult to follow patches.

Lastly, each new version of a patch must contain changelog. I don't see changelog here, please add it to v3 after the " ---" separator of the commit message. Thanks!

Example:

Commit message.

Signed-off-by: Akhil R akhilrajeev@nvidia.com --- drivers/i2c/busses/i2c-tegra.c | 52 ++++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 12 deletions(-)

Changelog:

v3: bla-bla-bla

v2: bla-bla-bla

diff ...