From: Stephen Boyd <swboyd@chromium.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
Rob Herring <robh+dt@kernel.org>,
Satya Priya Kakitapalli <quic_c_skakit@quicinc.com>
Cc: Lee Jones <lee.jones@linaro.org>,
Liam Girdwood <lgirdwood@gmail.com>,
Mark Brown <broonie@kernel.org>,
linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, quic_collinsd@quicinc.com,
quic_subbaram@quicinc.com, quic_jprakash@quicinc.com
Subject: Re: [PATCH V13 7/9] regulator: Add a regulator driver for the PM8008 PMIC
Date: Fri, 27 May 2022 17:02:37 -0400 [thread overview]
Message-ID: <CAE-0n539gePyXhw7r+XcaHtooN98KfYsx_qwgDaFkJtMSg+80g@mail.gmail.com> (raw)
In-Reply-To: <e70aceba-02d5-15b5-46d0-d5ed5706e81a@quicinc.com>
HTML mail? Please send plaintext next time.
Quoting Satya Priya Kakitapalli (Temp) (2022-05-27 01:24:19)
>
> On 5/21/2022 8:26 AM, Stephen Boyd wrote:
>
> Quoting Satya Priya (2022-05-20 03:49:35)
>
> diff --git a/drivers/regulator/qcom-pm8008-regulator.c b/drivers/regulator/qcom-pm8008-regulator.c
> new file mode 100644
> index 0000000..6e815c6
> --- /dev/null
> +++ b/drivers/regulator/qcom-pm8008-regulator.c
> @@ -0,0 +1,225 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2022, The Linux Foundation. All rights reserved. */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/qcom_pm8008.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>
> What in of_device.h is used?
>
>
> struct of_device_id
That struct is defined in mod_devicetable.h, not of_device.h
>
>
>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +
[...]
>
>
> +};
> +
> +static int pm8008_regulator_get_voltage(struct regulator_dev *rdev)
> +{
> + struct pm8008_regulator *pm8008_reg = rdev_get_drvdata(rdev);
> +
> + return pm8008_reg->voltage_selector;
>
> Can this read the hardware instead of caching the value from
> pm8008_regulator_set_voltage()?
>
>
> I can use the regmap_bulk_read like below (which was present in the earlier
> versions)
Please do
> + }
> +
> + pm8008_reg->dev = dev;
> +
> + rc = of_property_read_string(dev->of_node, "regulator-name", &name);
> + if (rc)
> + return rc;
> +
> + /* get the required regulator data */
> + for (i = 0; i < ARRAY_SIZE(reg_data); i++)
> + if (strstr(name, reg_data[i].name))
>
> Why not find this via reg/address instead? It would save storing the
> regulator name in the reg_data table.
>
>
> You mean match this using base address? then we should add base address in the
> reg_data table. We will need the name to be stored in reg_data table anyway for
> the pm8008_reg->rdesc.of_match
Why? Now that this driver binds to each node individually the usage of
of_match doesn't make any sense to me. Can you set 'struct
regulator_config::dev' instead and not set of_match?
next prev parent reply other threads:[~2022-05-27 21:02 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-20 10:49 [PATCH V13 0/9] Add Qualcomm Technologies, Inc. PM8008 regulator driver Satya Priya
2022-05-20 10:49 ` [PATCH V13 1/9] dt-bindings: mfd: pm8008: Add reset-gpios Satya Priya
2022-05-20 10:49 ` [PATCH V13 2/9] dt-bindings: mfd: pm8008: Change the address cells Satya Priya
2022-05-20 10:49 ` [PATCH V13 3/9] dt-bindings: mfd: pm8008: Add regulators for pm8008 Satya Priya
2022-05-20 10:49 ` [PATCH V13 4/9] mfd: pm8008: Add reset-gpios Satya Priya
2022-05-20 10:49 ` [PATCH V13 5/9] mfd: pm8008: Remove the regmap member from pm8008_data struct Satya Priya
2022-05-20 10:49 ` [PATCH V13 6/9] mfd: pm8008: Use i2c_new_dummy_device() API Satya Priya
2022-05-20 10:49 ` [PATCH V13 7/9] regulator: Add a regulator driver for the PM8008 PMIC Satya Priya
2022-05-21 2:56 ` Stephen Boyd
[not found] ` <e70aceba-02d5-15b5-46d0-d5ed5706e81a@quicinc.com>
2022-05-27 21:02 ` Stephen Boyd [this message]
2022-05-30 10:33 ` Satya Priya Kakitapalli (Temp)
2022-06-01 2:06 ` Stephen Boyd
2022-06-01 11:18 ` Satya Priya Kakitapalli (Temp)
2022-05-20 10:49 ` [PATCH V13 8/9] arm64: dts: qcom: pm8008: Add base dts file Satya Priya
2022-05-20 10:49 ` [PATCH V13 9/9] arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp Satya Priya
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAE-0n539gePyXhw7r+XcaHtooN98KfYsx_qwgDaFkJtMSg+80g@mail.gmail.com \
--to=swboyd@chromium.org \
--cc=bjorn.andersson@linaro.org \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=lee.jones@linaro.org \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=quic_c_skakit@quicinc.com \
--cc=quic_collinsd@quicinc.com \
--cc=quic_jprakash@quicinc.com \
--cc=quic_subbaram@quicinc.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).