Date:   Mon, 20 Mar 2023 17:33:44 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Khadija Kamran <kamrankhadijadj@gmail.com>,
        Ira Weiny <ira.weiny@intel.com>
CC:     <outreachy@lists.linux.dev>,
        Vaibhav Hiremath <hvaibhav.linux@gmail.com>,
        Johan Hovold <johan@kernel.org>, Alex Elder <elder@kernel.org>,
        "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
        <greybus-dev@lists.linaro.org>, <linux-staging@lists.linux.dev>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] staging: greybus: remove tabs to fix line length and
 merge lines
Message-ID: <6418fb683bc37_2c7706294e3@iweiny-mobl.notmuch>
References: <ZAusnKYVTGvO5zoi@khadija-virtual-machine>
 <640e75cfd8fc_229a89294a3@iweiny-mobl.notmuch>
 <ZBge/v/K/BOVkl2V@khadija-virtual-machine>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ZBge/v/K/BOVkl2V@khadija-virtual-machine>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SGFDV3N6N0pmU2owTk4xb25ZN2Y0LzZvU1J2ZWFRL3ArcnVGNWljc210czJX?=
 =?utf-8?B?RWFETGxocGw0cXowejkwZ0Y5dmZBeE5SWkFtM0lEN0JpSExEWHR1cmkySm9Y?=
 =?utf-8?B?VEdibWMrUzNMV09VQ2l1VGppNEpneDNucEUwM2VpYUtESk5ocGxKL3hpOUdM?=
 =?utf-8?B?ZTB1Y0twVGJxQk85YW5vR1lQcE5zeGJGeG5FTGNMYnJsU2t4THp5SEl6cDFB?=
 =?utf-8?B?Snd3R2NObTVjeDR0Yy9TTVJlMXEzbkdsZWkzK3JUNENVallZekp3TGhVaURH?=
 =?utf-8?B?SEdxL1NFUkZWZkIwdVdJb2MyZFEzcm1HelFEMEtKTmtadEJsSDZlRCsyak5z?=
 =?utf-8?B?czlJTDlPcjVmOG9TT2F5aFlUelRzdXZMMnpLY0JOQUpqckFoRnRWekZBM2s4?=
 =?utf-8?B?dFpZZzFsM2JlenhzbFQvRW40bEUyZjZ3bGJrVld3OXZyK2VhWGZ0MUxIUzJR?=
 =?utf-8?B?NjB3ZEJGUGlBaXBWNm0vNGRNQkJjNURNcFRmOEpOSnNrbWx6d0lKUkM3OXlC?=
 =?utf-8?B?V3ZTUEVCVGV2R0RSTVloTUpVOTU0Yis5bTRyckp3Y0w4TlFRdGpEcytNUUlJ?=
 =?utf-8?B?ZHpLWFZrYTJQOWJDOVVEN0c0MC9CRjVXTVJkN2NVTEFZNnVVUWNOblBKT0p6?=
 =?utf-8?B?SHdvNk9iTkZoaXl2L1BzUTlHbCt2cVd1OWVXNUg2bFY5REdGWTVUZUFuVEVk?=
 =?utf-8?B?RVhKb1R2dVB6d2U4eEh2dWVLS3hTR1g0c2RQMVpDU2JadGx1ekgzbVFQWnZ0?=
 =?utf-8?B?ZDZuOW9FS3ZyOFFUZVF3MGxVaEJBbEE3K1BRdlpSWkxYODVDRzZINXIzR2ZK?=
 =?utf-8?B?bEhud2VBd3orbG1QZFlIZW94dUU0Rlo3akQ2RHdidkd0cno1bTVKSzFjS0Nw?=
 =?utf-8?B?aUdDWmZSVFhicFRsbE14azM4cnJDbHBrZEl3NDNZNjI2Yklvc09mbEF4czFH?=
 =?utf-8?B?S0RVTGV3UzRWNGVLRXFXTzUrKzlDeWx3YUw0YmVsQzRHU0NTZjhaVHpKTlg1?=
 =?utf-8?B?YnZ0TkkyS09oVEM3RVJaV0JYUHV5Mnk1aWZlTkFicWx3SllEQ2l1V0U5WUdN?=
 =?utf-8?B?MFkweU5NYkNwM3hraVo0dHYvb1lPc2RtS1BweUJDTytwcFZBYVdLU01sOHpW?=
 =?utf-8?B?MXc1TlloVml4Y1J3U2xqRmNhcFl0aXJiL2FpUlJCWWhSU3RYZXNEb1N0T2dX?=
 =?utf-8?B?L2VsajFBYnVSN3NMaVhxaG1QZTVoZ21jamJCcFR0ZXdiYUlwVjFGcFNtblho?=
 =?utf-8?B?U0ZwOWlrbkQxWkw4ZzVYaTNreU5FUXJZYnFmMHNzZG04WWJwbVNldEZPbzVR?=
 =?utf-8?B?UEJBamtKMFRPaGVhK0lBQTlXc0FSaXF4WCtjN1NqOUdaM2x1dThJTFlWTW12?=
 =?utf-8?B?a3dZVk1QWmVaVGZRdkE4QlVFOFRtbVhCcXloY0V1NkMvNWJobnQzOUc3dTFk?=
 =?utf-8?B?bjBTQzlzWDFNOHhialRiK2lybWVCWEZTQWxwWHplRXYrSktrT2kreXdHVkdL?=
 =?utf-8?B?S1FaUG1IQU93Vzc0RkcyZlJ6aTVWanpCTWV1dGRtYVBld0tDMkVqTnRrRVNR?=
 =?utf-8?B?Tzk2SDYxQkFMOE0rRkFjQmpMbU1Xbi9TWG5ZT2V1cGJuUGp2WHRoMC9tYXVk?=
 =?utf-8?B?TjViOGhvNTRNUUZJZjlaNldML0NHNlFJek12OFI1a1F4V244TjAyY1luOFhl?=
 =?utf-8?B?RXRuTjN6NHA3OStKQ2FuWXFBVnNCeXJZVXdhcjc1OU80eFFQbXYxVVlTRExU?=
 =?utf-8?B?R3l1RTFMSHpOY3JUR1NiMi9yWmVCRC9GVTBZTk0rTU5nbDUwNTVoN2l2S09Q?=
 =?utf-8?B?T0JtZElSay94a0RGK251STVYQkpPTDc3WFMvK0loU2hVSzhpc2Y0N3B5WDJa?=
 =?utf-8?B?YzY3WVhLcHpVV3Q3bEVIOHdqRzJsejJBV0VielBNMUxIRy9waG5qYy83TDB4?=
 =?utf-8?B?emluaDVjMVVzeHhMUjJmRHJRZnZsWXF1SXU4WVdRazBya0xscHlVaVRBNTlI?=
 =?utf-8?B?aDdwN0JHZlU0L3ZLak56VUMzR00wcktGUnRkSTUwajAva1VvdmJaQUc2NUlq?=
 =?utf-8?B?eXQ3Und5VzAwb1pWR3U2YjJZcjMrdmZLMm5MbWh1N2t2eGoyamt5b2Q5TGhp?=
 =?utf-8?B?UFBsQXJ4ZGJGQm12dGN2Tit0YktybFFWRUV1TnFsbHFOM2RSbFB5VGVjR1Zh?=
 =?utf-8?Q?dA5G11yS/dOMtZeIUvdYbLWU4rRWXIkdj97EQ73cyZ6f?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 524a68a0-8dfe-40e7-d602-08db29a3ef61
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Mar 2023 00:33:47.7818
 (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: 1C40T0un3m/sdi/yYKBx7XG2OxhqxBl1IEuALGQLenQkz1g+I8jiJWA1cSarkoYz9GpNZ6+DcPrHosdw7mzSyQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB5775
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Khadija Kamran wrote:
> On Sun, Mar 12, 2023 at 06:01:03PM -0700, Ira Weiny wrote:
> > Khadija Kamran wrote:
> > > In file drivers/staging/greybus/arche-platform.c,
> > > - Length of line 181 exceeds 100 columns, fix by removing tabs from the
> > >   line.
> > > - If condition and spin_unlock_...() call is split into two lines, join
> > > them to form a single line.
> > > 
> > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > 
> > Fundamentally the problem with arche_platform_wd_irq() is that the
> > indentation is too great.
> > 
> > "... if you need more than 3 levels of indentation, you’re screwed anyway,
> > and should fix your program."
> > 
> > 	-- https://www.kernel.org/doc/html/v4.10/process/coding-style.html#indentation
> > 
> > I think a better solution would be to refactor the entire function.  This
> > would make the logic of the function more clear as well IMHO.
> > 
> > Here is another tip to help:
> > 
> > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#centralized-exiting-of-functions
> > 
> > Do you think you could try that?
> 
> Hey Ira!
> 
> Sorry about the late reply. Thank you for your feedback. I have looked
> into the above link. Are you referring to the use of goto statements in
> arche_platform_wd_irq() call?

I'm not quite sure I understand what you mean because currently
arche_platform_wd_irq() does not use gotos.

But I think you are asking if I think it should.  If that is what you mean
then 'yes' I think arche_platform_wd_irq() could use goto's to clean up
the flow.

Ira