Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200725181404.18951-1-ansuelsmth@gmail.com> <20200725181404.18951-3-ansuelsmth@gmail.com>
In-Reply-To: <20200725181404.18951-3-ansuelsmth@gmail.com>
From:   Amit Kucheria <amit.kucheria@linaro.org>
Date:   Tue, 11 Aug 2020 18:27:53 +0530
Message-ID: <CAHLCerN_S+5G0oZrAjoui7U6H+H46uLeYHe3p3PVbmCDwJtC_Q@mail.gmail.com>
Subject: Re: [RFC PATCH v5 2/7] drivers: thermal: tsens: Convert msm8960 to reg_field
To:     Ansuel Smith <ansuelsmth@gmail.com>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Zhang Rui <rui.zhang@intel.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Linux PM list <linux-pm@vger.kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Jul 25, 2020 at 11:44 PM Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> Covert msm9860 driver to reg_filed to use the init_common

typo: field

> function.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/thermal/qcom/tsens-8960.c | 74 +++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
>
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index 2a28a5af209e..45cd0cdff2f5 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -56,6 +56,18 @@
>  #define TRDY_MASK              BIT(7)
>  #define TIMEOUT_US             100
>
> +#define S0_STATUS_OFF          0x3628
> +#define S1_STATUS_OFF          0x362c
> +#define S2_STATUS_OFF          0x3630
> +#define S3_STATUS_OFF          0x3634
> +#define S4_STATUS_OFF          0x3638
> +#define S5_STATUS_OFF          0x3664  /* Sensors 5 thru 10 found on apq8064/msm8960 */

Run checkpatch and fix spelling. :-) Or just say 'sensor 5-10'

> +#define S6_STATUS_OFF          0x3668
> +#define S7_STATUS_OFF          0x366c
> +#define S8_STATUS_OFF          0x3670
> +#define S9_STATUS_OFF          0x3674
> +#define S10_STATUS_OFF         0x3678
> +
>  static int suspend_8960(struct tsens_priv *priv)
>  {
>         int ret;
> @@ -269,6 +281,66 @@ static int get_temp_8960(const struct tsens_sensor *s, int *temp)
>         return -ETIMEDOUT;
>  }
>
> +static struct tsens_features tsens_8960_feat = {
> +       .ver_major      = VER_0,
> +       .crit_int       = 0,
> +       .adc            = 1,
> +       .srot_split     = 0,
> +       .max_sensors    = 11,

Align the equal to like in other files please.

> +};
> +
> +static const struct reg_field tsens_8960_regfields[MAX_REGFIELDS] = {
> +       /* ----- SROT ------ */
> +       /* No VERSION information */
> +
> +       /* CNTL */
> +       [TSENS_EN]     = REG_FIELD(CNTL_ADDR,  0, 0),
> +       [TSENS_SW_RST] = REG_FIELD(CNTL_ADDR,  1, 1),
> +       /* 8960 has 5 sensors, 8660 has 11, we only handle 5 */
> +       [SENSOR_EN]    = REG_FIELD(CNTL_ADDR,  3, 7),
> +
> +       /* ----- TM ------ */
> +       /* INTERRUPT ENABLE */
> +       // [INT_EN] = REG_FIELD(TM_INT_EN_OFF, 0, 0),

Get rid of these comments and at the very least use C-style comments.

> +
> +       /* Single UPPER/LOWER TEMPERATURE THRESHOLD for all sensors */
> +       [LOW_THRESH_0]   = REG_FIELD(THRESHOLD_ADDR,  0,  7),
> +       [UP_THRESH_0]    = REG_FIELD(THRESHOLD_ADDR,  8, 15),
> +       [MIN_THRESH_0]   = REG_FIELD(THRESHOLD_ADDR, 16, 23),
> +       [MAX_THRESH_0]   = REG_FIELD(THRESHOLD_ADDR, 24, 31),
> +
> +       // /* UPPER/LOWER INTERRUPT [CLEAR/STATUS] */
> +       // /* 1 == clear, 0 == normal operation */

Get rid of these comments and at the very least use C-style comments.


> +       [LOW_INT_CLEAR_0]   = REG_FIELD(CNTL_ADDR,  9,  9),
> +       [UP_INT_CLEAR_0]    = REG_FIELD(CNTL_ADDR, 10, 10),
> +
> +       /* NO CRITICAL INTERRUPT SUPPORT on 8960 */
> +
> +       /* Sn_STATUS */
> +       [LAST_TEMP_0]  = REG_FIELD(S0_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_1]  = REG_FIELD(S1_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_2]  = REG_FIELD(S2_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_3]  = REG_FIELD(S3_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_4]  = REG_FIELD(S4_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_5]  = REG_FIELD(S5_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_6]  = REG_FIELD(S6_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_7]  = REG_FIELD(S7_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_8]  = REG_FIELD(S8_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_9]  = REG_FIELD(S9_STATUS_OFF,  0,  7),
> +       [LAST_TEMP_10] = REG_FIELD(S10_STATUS_OFF, 0,  7),
> +
> +       /* No VALID field on 8960 */
> +       /* TSENS_INT_STATUS bits: 1 == threshold violated */
> +       [MIN_STATUS_0] = REG_FIELD(INT_STATUS_ADDR, 0, 0),
> +       [LOWER_STATUS_0] = REG_FIELD(INT_STATUS_ADDR, 1, 1),
> +       [UPPER_STATUS_0] = REG_FIELD(INT_STATUS_ADDR, 2, 2),
> +       /* No CRITICAL field on 8960 */
> +       [MAX_STATUS_0] = REG_FIELD(INT_STATUS_ADDR, 3, 3),
> +
> +       /* TRDY: 1=ready, 0=in progress */
> +       [TRDY] = REG_FIELD(INT_STATUS_ADDR, 7, 7),
> +};
> +
>  static const struct tsens_ops ops_8960 = {
>         .init           = init_8960,
>         .calibrate      = calibrate_8960,
> @@ -282,4 +354,6 @@ static const struct tsens_ops ops_8960 = {
>  struct tsens_plat_data data_8960 = {
>         .num_sensors    = 11,
>         .ops            = &ops_8960,
> +       .feat           = &tsens_8960_feat,
> +       .fields         = tsens_8960_regfields,
>  };
> --
> 2.27.0
>