From: Lee Jones <lee.jones@linaro.org>
To: Saurabh Sengar <saurabh.truth@gmail.com>
Cc: galak@codeaurora.org, agross@codeaurora.org,
davidb@codeaurora.org, linux-arm-msm@vger.kernel.org,
linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mfd: ssbi: removing unnecessary strcmp
Date: Mon, 16 Nov 2015 12:50:48 +0000 [thread overview]
Message-ID: <20151116125048.GC17829@x1> (raw)
In-Reply-To: <1447582760-6088-1-git-send-email-saurabh.truth@gmail.com>
On Sun, 15 Nov 2015, Saurabh Sengar wrote:
> passing the actual enum value for controller type from device tree,
> instead of passing the string and converting it to enum
Initially I would have probably preferred this version, but I can't
think of a good enough reason to break the current ABI in order to
replace it.
> Signed-off-by: Saurabh Sengar <saurabh.truth@gmail.com>
> ---
> Documentation/devicetree/bindings/arm/msm/ssbi.txt | 6 +++---
> arch/arm/boot/dts/qcom-apq8064.dtsi | 3 ++-
> arch/arm/boot/dts/qcom-ipq8064.dtsi | 3 ++-
> arch/arm/boot/dts/qcom-msm8660.dtsi | 3 ++-
> arch/arm/boot/dts/qcom-msm8960.dtsi | 3 ++-
> drivers/mfd/ssbi.c | 17 ++++++-----------
> include/dt-bindings/mfd/qcom,ssbi.h | 19 +++++++++++++++++++
> 7 files changed, 36 insertions(+), 18 deletions(-)
> create mode 100644 include/dt-bindings/mfd/qcom,ssbi.h
>
> diff --git a/Documentation/devicetree/bindings/arm/msm/ssbi.txt b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
> index 54fd5ce..61a37e0 100644
> --- a/Documentation/devicetree/bindings/arm/msm/ssbi.txt
> +++ b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
> @@ -10,9 +10,9 @@ These require the following properties:
>
> - qcom,controller-type
> indicates the SSBI bus variant the controller should use to talk
> - with the slave device. This should be one of "ssbi", "ssbi2", or
> - "pmic-arbiter". The type chosen is determined by the attached
> - slave.
> + with the slave device. This should be one of MSM_SBI_CTRL_SSBI,
> + MSM_SBI_CTRL_SSBI2 or MSM_SBI_CTRL_PMIC_ARBITER.
> + The type chosen is determined by the attached slave.
>
> The slave device should be the single child node of the ssbi device
> with a compatible field.
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index a4c1762..e391a01 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -5,6 +5,7 @@
> #include <dt-bindings/reset/qcom,gcc-msm8960.h>
> #include <dt-bindings/clock/qcom,mmcc-msm8960.h>
> #include <dt-bindings/soc/qcom,gsbi.h>
> +#include <dt-bindings/mfd/qcom,ssbi.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> / {
> model = "Qualcomm APQ8064";
> @@ -339,7 +340,7 @@
> qcom,ssbi@500000 {
> compatible = "qcom,ssbi";
> reg = <0x00500000 0x1000>;
> - qcom,controller-type = "pmic-arbiter";
> + qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;
>
> pmicintc: pmic@0 {
> compatible = "qcom,pm8921";
> diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> index fa69863..a3ba13a 100644
> --- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> @@ -3,6 +3,7 @@
> #include "skeleton.dtsi"
> #include <dt-bindings/clock/qcom,gcc-ipq806x.h>
> #include <dt-bindings/clock/qcom,lcc-ipq806x.h>
> +#include <dt-bindings/mfd/qcom,ssbi.h>
> #include <dt-bindings/soc/qcom,gsbi.h>
>
> / {
> @@ -307,7 +308,7 @@
> qcom,ssbi@500000 {
> compatible = "qcom,ssbi";
> reg = <0x00500000 0x1000>;
> - qcom,controller-type = "pmic-arbiter";
> + qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;
> };
>
> gcc: clock-controller@900000 {
> diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi
> index e5f7f33..51b28d2 100644
> --- a/arch/arm/boot/dts/qcom-msm8660.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi
> @@ -4,6 +4,7 @@
>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/clock/qcom,gcc-msm8660.h>
> +#include <dt-bindings/mfd/qcom,ssbi.h>
> #include <dt-bindings/soc/qcom,gsbi.h>
>
> / {
> @@ -112,7 +113,7 @@
> qcom,ssbi@500000 {
> compatible = "qcom,ssbi";
> reg = <0x500000 0x1000>;
> - qcom,controller-type = "pmic-arbiter";
> + qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;
>
> pmicintc: pmic@0 {
> compatible = "qcom,pm8058";
> diff --git a/arch/arm/boot/dts/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom-msm8960.dtsi
> index 134cd91..d01cee5 100644
> --- a/arch/arm/boot/dts/qcom-msm8960.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8960.dtsi
> @@ -5,6 +5,7 @@
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/clock/qcom,gcc-msm8960.h>
> #include <dt-bindings/mfd/qcom-rpm.h>
> +#include <dt-bindings/mfd/qcom,ssbi.h>
> #include <dt-bindings/soc/qcom,gsbi.h>
>
> / {
> @@ -171,7 +172,7 @@
> qcom,ssbi@500000 {
> compatible = "qcom,ssbi";
> reg = <0x500000 0x1000>;
> - qcom,controller-type = "pmic-arbiter";
> + qcom,controller-type = <MSM_SBI_CTRL_PMIC_ARBITER>;
>
> pmicintc: pmic@0 {
> compatible = "qcom,pm8921";
> diff --git a/drivers/mfd/ssbi.c b/drivers/mfd/ssbi.c
> index 27986f6..cf4d983 100644
> --- a/drivers/mfd/ssbi.c
> +++ b/drivers/mfd/ssbi.c
> @@ -274,7 +274,7 @@ static int ssbi_probe(struct platform_device *pdev)
> struct device_node *np = pdev->dev.of_node;
> struct resource *mem_res;
> struct ssbi *ssbi;
> - const char *type;
> + u32 type;
>
> ssbi = devm_kzalloc(&pdev->dev, sizeof(*ssbi), GFP_KERNEL);
> if (!ssbi)
> @@ -287,22 +287,17 @@ static int ssbi_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, ssbi);
>
> - type = of_get_property(np, "qcom,controller-type", NULL);
> - if (type == NULL) {
> + if (of_property_read_u32(np, "qcom,controller-type", &type)) {
> dev_err(&pdev->dev, "Missing qcom,controller-type property\n");
> return -EINVAL;
> }
> - dev_info(&pdev->dev, "SSBI controller type: '%s'\n", type);
> - if (strcmp(type, "ssbi") == 0)
> - ssbi->controller_type = MSM_SBI_CTRL_SSBI;
> - else if (strcmp(type, "ssbi2") == 0)
> - ssbi->controller_type = MSM_SBI_CTRL_SSBI2;
> - else if (strcmp(type, "pmic-arbiter") == 0)
> - ssbi->controller_type = MSM_SBI_CTRL_PMIC_ARBITER;
> - else {
> +
> + dev_info(&pdev->dev, "SSBI controller type: '%d'\n", type);
> + if (type > MSM_SBI_CTRL_PMIC_ARBITER) {
> dev_err(&pdev->dev, "Unknown qcom,controller-type\n");
> return -EINVAL;
> }
> + ssbi->controller_type = type;
>
> if (ssbi->controller_type == MSM_SBI_CTRL_PMIC_ARBITER) {
> ssbi->read = ssbi_pa_read_bytes;
> diff --git a/include/dt-bindings/mfd/qcom,ssbi.h b/include/dt-bindings/mfd/qcom,ssbi.h
> new file mode 100644
> index 0000000..b6bf5bc
> --- /dev/null
> +++ b/include/dt-bindings/mfd/qcom,ssbi.h
> @@ -0,0 +1,19 @@
> +/* Copyright (c) 2015, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#ifndef __DT_MFD_QCOM_SSBI_H
> +#define __DT_MFD_QCOM_SSBI_H
> +
> +#define MSM_SBI_CTRL_SSBI 0
> +#define MSM_SBI_CTRL_SSBI2 1
> +#define MSM_SBI_CTRL_PMIC_ARBITER 2
> +
> +#endif
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
prev parent reply other threads:[~2015-11-16 12:50 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-15 10:19 [PATCH] mfd: ssbi: removing unnecessary strcmp Saurabh Sengar
2015-11-16 12:50 ` Lee Jones [this message]
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=20151116125048.GC17829@x1 \
--to=lee.jones@linaro.org \
--cc=agross@codeaurora.org \
--cc=davidb@codeaurora.org \
--cc=galak@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-soc@vger.kernel.org \
--cc=saurabh.truth@gmail.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.