Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Jadav, Raag" <raag.jadav@intel.com>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
CC:     "linus.walleij@linaro.org" <linus.walleij@linaro.org>,
        "mika.westerberg@linux.intel.com" <mika.westerberg@linux.intel.com>,
        "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Sangannavar, Mallikarjunappa" 
        <mallikarjunappa.sangannavar@intel.com>,
        "N, Pandith" <pandith.n@intel.com>
Subject: RE: [PATCH v4] pinctrl: intel: refine ->irq_set_type() hook
Thread-Topic: [PATCH v4] pinctrl: intel: refine ->irq_set_type() hook
Thread-Index: AQHZn4gBPXOZzjOwf0Shiugls1EKsK+L15oAgAACIqCAAA0ZgIAADlVg
Date:   Thu, 15 Jun 2023 15:05:42 +0000
Message-ID: <DM6PR11MB2779545C0ACBF833E857EBD28C5BA@DM6PR11MB2779.namprd11.prod.outlook.com>
References: <20230615125022.27421-1-raag.jadav@intel.com>
 <ZIsOvBGLJTCo45jp@smile.fi.intel.com>
 <DM6PR11MB2779FD7F6DF3A0126DB85E4B8C5BA@DM6PR11MB2779.namprd11.prod.outlook.com>
 <ZIsbg2h0u93tCQiz@smile.fi.intel.com>
In-Reply-To: <ZIsbg2h0u93tCQiz@smile.fi.intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?0IBNbLXMncwxDpJxbCpr9KqSW2IucPdwNE6Abpnq4qV7ooX6ubvHnmBQWpyh?=
 =?us-ascii?Q?fryLLhxRedPIyY4YtzkhW6l7P/AhV9K2gK9MLFmWwrHp5z3BWoN+JZ58EXX8?=
 =?us-ascii?Q?NBW9/s1llbpzLiJb0us+lN7P+cK69cFi8PKgs0HqYmaIwBpsQrhfe7h+MpFJ?=
 =?us-ascii?Q?bXGSNauue5Uc7dH0tvNZsnY5W02tpVJNsiamLM3qv3wsc5ZKtQaOxRInDvEU?=
 =?us-ascii?Q?dx7P+q4eON2XR1d/2/PizSRXunKuKR+YPjEACUR2enGmMVU16JCJLJEy2KND?=
 =?us-ascii?Q?ehXKTJ4oqoLtjJqudgAdILgyt8VVtO3pikkYqkZ7T+r04b1cTi0rGPXFn+ee?=
 =?us-ascii?Q?CWqgsqLYCm4MrOMZAD9uecENJWOUezieY/Y0qgnklAMvZ87Tsv2PF7/e736K?=
 =?us-ascii?Q?+a/+xLzAo1Woc7hsZ+bWj8ioM72DVikBQVSs4WPTL/RIIVFR8/6TE/TNTNPs?=
 =?us-ascii?Q?pRtObhmTd4LWncFxp5NMd1iXRvwm72SclUrhw4Yd8V+CRuaQwk61/sdYKg25?=
 =?us-ascii?Q?Te4pcIQ2PQl9BL0yGM9GJytgrNauHm44qA95gEHsdd5pEGVutSZIZqvJAEZR?=
 =?us-ascii?Q?QRBWQw3aHiic23Z25PadeeHlqhxiNiqm+9Q/0QGsLT8aZYwGyM2Zg1NcJhpW?=
 =?us-ascii?Q?J+4mw4tVn9uVnzzwUcdJ7yoVTEgmIJwsu6jxJZwWHXvRHFOIkO6Oks8p2/+Q?=
 =?us-ascii?Q?3Ej06EtPaHehv1JBHpBkXLDH1rsaC+HL5qQY5Mn+9rpOSg1yXA0hviAowfUG?=
 =?us-ascii?Q?DOeAs3aPAZWMWETVoYE5fK23NzwGe/LpiVS1OP94QdfHJgmC5Yh3MzwJnhx+?=
 =?us-ascii?Q?UXPtf5h/JMkxHkav/DYr0QXwhfmMAHxbrBOSIBlBSmf0enISgppJYWNJjccD?=
 =?us-ascii?Q?qYcC0x15FEQMdJIRJzXijju8q7A7OyCqhslxC0g4tBnLsphZF24UMRpl8cBF?=
 =?us-ascii?Q?xkEWOGXUJLelRj5S/VGdIdlBc69RxJ22ryLSQeZo6hChb/Z5QAVzUXs6nAnP?=
 =?us-ascii?Q?shq4yTv5tNRSxakhmmLd47lqbmFFTyr1uhSypNxtNHiMT40kVulGAgIsqDli?=
 =?us-ascii?Q?hmkI7LsgkKKEpT7pyGPKx6wgZvmDcAGm9XoELWDog3qnNEDNJuEmtIgB59o3?=
 =?us-ascii?Q?MTAdjFB4qkRmy3VwuyznJ4rQnl6lUPw+aGZZBcDc1Q4kIx+4mFdaIEcRaDgP?=
 =?us-ascii?Q?LeIQBkmc801QTJ/quB5kSEJey++tsNUF6AJeRJUckzyUnkihdaeGPrhj5UUx?=
 =?us-ascii?Q?WDEcCldW4KDenppzg3KFdwqNkkyg23E0dBmcUIK1iKofPutUQA7ARIOzfExA?=
 =?us-ascii?Q?pBoHdWCkT7K47V8xERlr7rwwWF+S4GublBw6qUeNN+VDpoaW3KF5bpS5ebdr?=
 =?us-ascii?Q?Ef+O+GS7koqB6ylI4q6afKJui5wBHnzSIoNxz138yeNgkduDySAGlEy4naS+?=
 =?us-ascii?Q?oFKco54gBjl9xTjk55scazarkl3drPCmofSXwRXR967lpyL+i1U9QNx9k9zf?=
 =?us-ascii?Q?+uHvmrsELCvKqV47NJwi/WbG0O9ksj6w9NPwDXHTLtPRGeywg2fOQA6ONNR2?=
 =?us-ascii?Q?MukUjKgEf8quSN71tj4=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB2779.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3cc2d4cd-7008-4d74-b5a3-08db6db1fd1a
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Jun 2023 15:05:42.5556
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: nrL8sxrgqKm9q1VG/LuaDv9YkhylOhmeSFvsBBxrCOAUGh7qNsL1XMGlhjy8ha/uDr86GTNPaAG1gmf/jOjJcg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6784
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> On Thu, Jun 15, 2023 at 01:35:19PM +0000, Jadav, Raag wrote:
> > > On Thu, Jun 15, 2023 at 06:20:22PM +0530, Raag Jadav wrote:
> > > > Refine ->irq_set_type() hook and improve its readability by:
> > > >
> > > > - Reducing scope of spinlock by moving unneeded operations out of i=
t.
> > > > - Dropping redundant PADCFG0_RXEVCFG_SHIFT and including it
> directly
> > > >   into PADCFG0_RXEVCFG_* definitions.
> > > > - Utilizing temporary variables for common operations.
> > > > - Simplifying if-else-if chain.
> > >
> > > Two questions out of curiosity.
> > > Do we gain or lose bytes with this?
> >
> > add/remove: 0/0 grow/shrink: 1/0 up/down: 33/0 (33)
> > Function                                     old     new   delta
> > intel_gpio_irq_type                          317     350     +33
> > Total: Before=3D10469, After=3D10502, chg +0.32%
> >
> > > > +	value =3D readl(reg);
> > >
> > > > +	value =3D (value & ~PADCFG0_RXINV) | rxinv;
> > > > +	value =3D (value & ~PADCFG0_RXEVCFG_MASK) | rxevcfg;
> > >
> > > Same question if we change this to be similar to the current code, i.=
e.
> > >
> > > 	value =3D readl(reg);
> > >
> > > 	value &=3D ~(PADCFG0_RXEVCFG_MASK | PADCFG0_RXINV);
> > > 	value |=3D rxevcfg;
> > > 	value |=3D rxinv;
> > >
> > > And I would keep blank lines after readl() and before writel() since
> > > we have more than a single line in between.
> >
> > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-9 (-9)
> > Function                                     old     new   delta
> > intel_gpio_irq_type                          317     308      -9
> > Total: Before=3D10469, After=3D10460, chg -0.09%
>=20
> Do I understand correctly that this is your patch + suggested above?

Yes, this is tested with gcc 7.5.0 with default -O2.
I see some reordering in disassembly even with this simple change,
and I'm not entirely sure what kind of weird tricks gcc is pulling here.