Date:   Tue, 21 Mar 2023 13:01:15 +0800
From:   Chester Lin <clin@suse.com>
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        NXP S32 Linux Team <s32@nxp.com>,
        linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>,
        Andrei Stefanescu <andrei.stefanescu@nxp.com>,
        Radu Pirea <radu-nicolae.pirea@nxp.com>,
        Andreas =?iso-8859-1?Q?F=E4rber?= <afaerber@suse.de>,
        Matthias Brugger <mbrugger@suse.com>
Subject: Re: [PATCH v2 3/4] pinctrl: s32cc: refactor pin config parsing
Message-ID: <ZBk6G61RUbFPIV3G@linux-8mug>
References: <20230320163823.886-1-clin@suse.com>
 <20230320163823.886-4-clin@suse.com>
 <CAHp75Ve325yfz1P4=vYndUOhV-VkqNCWVEObm1RURbKHgcLKrw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAHp75Ve325yfz1P4=vYndUOhV-VkqNCWVEObm1RURbKHgcLKrw@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V1ZwbXVOUm8xc1IrNENGUjhWS2RnNTVuZUhVdldJbjB6WllTZGQ4a0JUNWsv?=
 =?utf-8?B?YzJnaXhWRkJlZWp5VThXMVdhMWtzQzlkNFd2N2FBSUptRmtSQmtLaU1lN3Mr?=
 =?utf-8?B?ek5XYXc1VElYQXRYdWZDa1c5VXpncjVFeEI2UGZraURUbUpBbEtMaUp1MHVh?=
 =?utf-8?B?eFFET2x0bDZUNFlFN2NjbFNjNjVTV21Ha2hsVzIxWFZnVEFSRjRvWVQ5MC9P?=
 =?utf-8?B?UTZKN0lRbWtabGxIeWplK1JDeUVZV2FCOG5jZEVhaDVIUUVTaTlvV3JYU2k0?=
 =?utf-8?B?aVFDRVEvYTJkdnozekdlNDFJaWZPN2s4YkxMU3FObitubURZM3lkQjU5NFpS?=
 =?utf-8?B?eU43WVRtR1c0N0twM0V5VFpCM2Z3dzF0TUMxNWo3MTZ4ZTBjbTd3VVZnRjR1?=
 =?utf-8?B?QjFJdWJhVGo2ZkEyY0lWb052a0I5U3hoeFd5VVVMVFMvY1JLRTg2ek1sclV1?=
 =?utf-8?B?S2FOR0ZTei9KbFRyWmhjNjFUdmlDN1RvT0dRTUYvK3JnL3QzQ3g5NzZvVjBk?=
 =?utf-8?B?aFNUMDJVYXU4bTFGdHdBZmRaZXRDZVFIM3p0UXhkWnY0Ui9kS01yVW80cTZN?=
 =?utf-8?B?YmxBYjU0clNiYmFjcVNJTTdyWWhweDFNS2JwTExCT3JGZlpSdXhvMW5OWHhM?=
 =?utf-8?B?eGNlem5xM1Q5dnlveG5ZL1FqQ241Wm45QnNwTkJtTG92SmVQZGd6b2xXQm9y?=
 =?utf-8?B?OXVyazBJRjRTeXZFdDU5NmVvd29MNUN5WU0yN3Q1d2hxQzZVdnJBeUtzeEpX?=
 =?utf-8?B?TjdJM3BROU1TSEtub3RtWkhQdTZkeVJyU1lTVVgzWDdFbEM0dnIrT0ViN3lG?=
 =?utf-8?B?c2ttUTdQcWZwbWRELzlhSWlUbC8zSkpOWk9yRHJQcGJaaEw2QnBpd0NISWZW?=
 =?utf-8?B?dXRsZ3lKeEdsbWpHbUU5K05SUEM0ZUxnaldDRzdXNkdNbGlxYzRuQTRFMS9R?=
 =?utf-8?B?YlQvZDR4b01jTFozVjM1UnBsUDh4SjU2NTNDZno1c1J6V0VwcWlzOS9meXNm?=
 =?utf-8?B?WlpnUkdLdmdIWWN6QjlEOXZZdTFZYndBMzd3RWFFeUhJS1hNRHgxMHVzWm1P?=
 =?utf-8?B?OXBpd1JwUGlCd2lZUVh0RDVQeXB4ZzdGYkpDUzFubVIzeEpuSGhkZHRYMVFt?=
 =?utf-8?B?ZDJNQTd5dWxIcHhRd1QyTDlHd2lxQm1JSGJiQitzUDU5TEowNWl1cGR3Mkh0?=
 =?utf-8?B?UGZDc1lHWlJyTWxYaGNmQnNpQU1ReHRQQ3RxdmVOeVZaMU9HdlVoUE1wNUp3?=
 =?utf-8?B?czJ0V3h6TVBnVm5GQmVseGZ5SnJiZHdQaStuVTFjeHlHZzVSL2hYRExWcENQ?=
 =?utf-8?B?a0JaRDB5UDUrUjhRajVrb0NWZ0NkR3J6Y0tiTmhWdU91dERDMWtFbjJQNmMr?=
 =?utf-8?B?S2UrbW8yTFJ4Mk9vcTE5YytveVU3SFZScGt3SHJqZnlWd0pXSmNuQVZvNHJq?=
 =?utf-8?B?cUU0MzQrZkxZejgxd3hPZTlvSFlwUU1tam5xOHFQSk15TERrVytmQm9VYjhx?=
 =?utf-8?B?RkVsMkNOZ3dRV2k2Q05qSDBlMmpmeHFydjIxNnBtMDBYYWNxRHJmbUdzSjcx?=
 =?utf-8?B?SHZuVWxEZ0tIdTR2bndqL0tyUmFNOWRyVjFoV3AvVUI1cHFnVlE1TE9uTEdS?=
 =?utf-8?B?czJmb0RNZW4zOEdtRm9vUFNJSWI5V3FRSUtUQkExZG1uNWZLeFVwK2U4aGhB?=
 =?utf-8?B?eitzVncxamFBcFI2TlB6eDlJNE9jK0hldDhIME1kL0kyMFM3S2RLZWFadVEv?=
 =?utf-8?B?bHZaSmJNVDJxMURnZ2RGQW9qT2UvaXpzd01IODJjZlYrTjIwdUx0WjZ3SkZJ?=
 =?utf-8?B?SmVaeWlEK0lINkNNNWJjSDU4d1EwTFVvdzViNFN1bjRvZGVXTThsWVFrY0JI?=
 =?utf-8?B?OU1sSFpuMEpxai8wQ3FDN3pkZG1DK3hoSVJGN2IweFUzazZqaW1PTzNLcHZr?=
 =?utf-8?B?V2M5dDhSLytUeHh6ZDdlL0k2aTdPazFYTXNIVTZmcHlXSUdJdWdXMUlFaXIr?=
 =?utf-8?B?Y3k1MVpFU1FHQXFWS29IUzlDdzdDWWVOblU3RC9xemNLWmY3ZDFmaGErcWt5?=
 =?utf-8?B?ZDZoZ3NtM21STnlqNmtiV0d1MzFaQUtDQm52YkE0N3l6YWI3NitidGo4WU1V?=
 =?utf-8?Q?iMzA=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e6b4a162-d957-4911-50bd-08db29c956bc
X-MS-Exchange-CrossTenant-AuthSource: VI1PR0402MB3439.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Mar 2023 05:01:32.5591
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Q4cW9C3qIUe+I8U0QBM6Rh/1bAUvUFvNFhYVMb9pT1cMR+wZJQpRHM70pR8Q3jGY
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DUZPR04MB10013
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Mar 20, 2023 at 07:06:53PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 20, 2023 at 6:39 PM Chester Lin <clin@suse.com> wrote:
> >
> > Move common codes into smaller inline functions and remove some argument
> > handlings that are not actually used by either S32 MSCR register or generic
> > config params.
> 
> ...
> 
> >         case PIN_CONFIG_OUTPUT_ENABLE:
> > -               if (arg)
> > -                       *config |= S32_MSCR_OBE;
> > -               else
> > -                       *config &= ~S32_MSCR_OBE;
> > +               *config |= S32_MSCR_OBE;
> >                 *mask |= S32_MSCR_OBE;
> >                 break;
> >         case PIN_CONFIG_INPUT_ENABLE:
> > -               if (arg)
> > -                       *config |= S32_MSCR_IBE;
> > -               else
> > -                       *config &= ~S32_MSCR_IBE;
> > +               *config |= S32_MSCR_IBE;
> >                 *mask |= S32_MSCR_IBE;
> >                 break;
> 
> Isn't it a regression here?
> Otherwise needs an explicit explanation in the commit message on
> what's going on here and why it's not a regression.

Oops, it's wrong implementation. The argument checks of OUTPUT_EN and INPUT_EN
shouldn't be dropped. Thanks for the reminder and I will fix it.

Regards,
Chester

> 
> ...
> 
> >         case PIN_CONFIG_BIAS_DISABLE:
> > -               *config &= ~(S32_MSCR_PUS | S32_MSCR_PUE);
> > -               *mask |= S32_MSCR_PUS | S32_MSCR_PUE;
> > +               s32_pin_set_pull(param, mask, config);
> >                 break;
> 
> This now can be unified with PU and PD cases above.
> 
> -- 
> With Best Regards,
> Andy Shevchenko