Messages in this thread | | | Date | Mon, 19 Feb 2024 20:14:47 +0100 | From | Niklas Cassel <> | Subject | Re: [PATCH v3] ata: ahci_ceva: fix error handling for Xilinx GT PHY support |
| |
Hello Radhey, Markus,
On Mon, Feb 19, 2024 at 06:42:49PM +0000, Pandey, Radhey Shyam wrote: > > -----Original Message----- > > From: Markus Elfring <Markus.Elfring@web.de> > > Sent: Monday, February 19, 2024 9:27 PM > > To: Niklas Cassel <cassel@kernel.org>; Pandey, Radhey Shyam > > <radhey.shyam.pandey@amd.com>; Damien Le Moal > > <dlemoal@kernel.org>; Jens Axboe <axboe@kernel.dk>; Simek, Michal > > <michal.simek@amd.com>; Philipp Zabel <p.zabel@pengutronix.de>; linux- > > ide@vger.kernel.org; kernel-janitors@vger.kernel.org > > Cc: LKML <linux-kernel@vger.kernel.org>; git (AMD-Xilinx) <git@amd.com> > > Subject: Re: [PATCH v3] ata: ahci_ceva: fix error handling for Xilinx GT PHY > > support > > > > > > Platform clock and phy error resources are not cleaned up in Xilinx GT > > PHY > > > > error path. > > > > > > > > To fix introduce the function ceva_ahci_platform_enable_resources() > > … > > > Applied: > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/log/?h=for- > > 6.8-fixes > > > > The error code “-EINVAL” was set before the statement “goto > > disable_resources” > > multiple times in the adjusted implementation of the function > > “ceva_ahci_probe”. > > I suggest to add a jump target so that a bit of exception handling > > can be better reused at the end of this function. > > > > > > How do you think about to apply the following script for the semantic > > patch language (Coccinelle software) accordingly? > > > > > > @replacement1@ > > identifier rc; > > @@ > > <+... > > if (...) > > { > > ... when != rc > > - rc = -EINVAL; > > goto > > - disable_resources > > + e_inval > > ; > > } > > ...+> > > return 0; > > + > > +e_inval: > > +rc = -EINVAL; > > disable_resources: > > ahci_platform_disable_resources(hpriv); > > > > @replacement2 disable neg_if, drop_else@ > > identifier replacement1.rc; > > statement is; > > @@ > > if (...) > > is > > else > > { > > ... when != rc > > - rc = -EINVAL; > > goto > > - disable_resources > > + e_inval > > ; > > } > > > > > Thanks for the suggestion. However, taking a look at the existing implementation > i think we should return error code *as is * from of_property_read() APIs. > and get rid of rc=-EINVAL reassignment itself. > > If it sounds ok, I can add it to my to-do list and send out a patch.
Sounds good to me.
Kind regards, Niklas
| |