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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxx>
> */
>
> +#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@xxxxxxxxxx>
---
drivers/i2c/busses/i2c-tegra.c | 52
++++++++++++++++++++++++++++++++----------
1 file changed, 40 insertions(+), 12 deletions(-)
Changelog:
v3: bla-bla-bla
v2: bla-bla-bla
diff ...
