Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
From:   Niklas Cassel <Niklas.Cassel@wdc.com>
To:     Damien Le Moal <dlemoal@kernel.org>
CC:     Ma Ke <make_ruc2021@163.com>,
        "linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] ata: sata_mv: aspeed: fix value check in
 mv_platform_probe()
Thread-Topic: [PATCH v2] ata: sata_mv: aspeed: fix value check in
 mv_platform_probe()
Thread-Index: AQHZ+1kU5B4BC+3D3EianOt6+pON8g==
Date:   Tue, 10 Oct 2023 09:06:41 +0000
Message-ID: <ZSUUIGgzcAPudwdK@x1-carbon>
References: <20231009124955.4046988-1-make_ruc2021@163.com>
 <2c877878-7e30-43d6-ba93-d37cc2fbb1ef@kernel.org>
In-Reply-To: <2c877878-7e30-43d6-ba93-d37cc2fbb1ef@kernel.org>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?7s/WW3MxqSqVQaIun1/c1yd0TTYIxU5C2rHYi2ygmFCpTYqa/vNMPHF5lNm3?=
 =?us-ascii?Q?U2QTdMqDkvurrXSbvQH0ECUDPgKOe2MTCKjFDBsxjIYt3a8XUzEmllitP09l?=
 =?us-ascii?Q?JAlGELKp2b3NyBznQITTtJX7DjtiNjbe7OZEPIBkBinXS1opgyVY4UJCGCwL?=
 =?us-ascii?Q?Z/xt3XZAgBYN0PDQqKbaHHfCER+X28MVmKdiYCjQymR924F9aGmLT+aFJOIT?=
 =?us-ascii?Q?QBRf5YGK4KnPRgyTG4cPkcQPy0cTuBKLnLsG0rz5rBoA3EcOpzw4GE4FA8bd?=
 =?us-ascii?Q?5vud2f3GYNKZLsfjY6Xy86hUkqZEaFPd4WwDObqVK95YkWSqa/IJbjQq+lgC?=
 =?us-ascii?Q?Bk3XDjDVZyYnz4+4RvWVqAPAZI2k6hLLv4MwgsFjtR1wVq2IsJaT7Wwvx++S?=
 =?us-ascii?Q?YHHk3DFFTlh7A+SRCBBKFD+sq9AfsKaBN9P6ywV7PNQRDwGC7OvCSTIm2UVu?=
 =?us-ascii?Q?OH2KP2KSaG6OR3Ohr0ubNEqjKz5JXhQrdQFe3F8tgRwdVOxO1Nww28zetg67?=
 =?us-ascii?Q?DbXEPlMGmYcozvLuhMS5h8YFeIfm4GfA5dt2n7jDSE8sWx0c8gDbn4tuQ8lE?=
 =?us-ascii?Q?b37IOl4mCJWrGIZgSE+LctNnHJsJ5LKnifHvivjsJYSq+SWMWXl+K8LpCeds?=
 =?us-ascii?Q?7lXI+5Zj2sBx1dkEu0hS/zgAm+USEhH43Ls/5xJd8UpdLFGg9JOGmcEDWAMq?=
 =?us-ascii?Q?oN6OefreH4NIX7IkHH2B3WXhtoeGtlVZajPIIJCyoQUCZmldH3/9Gsob0870?=
 =?us-ascii?Q?fObgYpgjIL5pL/fdAgUUQaaRSGgxS2AdOc2FmzJMGDsCRLbfUkae5spdB0h/?=
 =?us-ascii?Q?b0FXhPtxw1kOP/FSHCpAcw9yeMQ4oXps/ce+vnbiUUbiNtH99TjOjHtR0f1p?=
 =?us-ascii?Q?VofBtef5LavLkdcMLiEP1JVw5gVmsG0yb6n/KseudTEK+bK3Mjv9s8ywG64s?=
 =?us-ascii?Q?OP8Nm1mi2s6n6DSCwKELd3tZzzWkEeRzYiziGKvKatdNnqEcUDfpbD4NnIQz?=
 =?us-ascii?Q?YR79+qi1aebGvOwUijP+CXOU9CdTJkMOHCMqFO5GmQ8+MPK/h6dPWdid69qI?=
 =?us-ascii?Q?isvTNM7VXU+VhnovM65YqhS0wYaGAqoT6rRdAaCcSYFlI025IKpP/GYQF7ZF?=
 =?us-ascii?Q?ZvfpzVrx2gn97MzDHXY9HiQdmMGvutgs68qoMtx/26e26TCNodqFCxnpFYWU?=
 =?us-ascii?Q?hUUgQoyfaseqznoo7eT9Vnx+yHpW/9/aR2Y/EnnN6uzSpzBft7hNt4QGNRko?=
 =?us-ascii?Q?oxLhLnl7xLa+71W6z4DmIkPQsJMjEzClTuUWO/dqLPRQpK5vrPY6cW3V/CI9?=
 =?us-ascii?Q?Dy2zmN+MRFc9ZNRLDVJwwzlcvDFZixwBCuKTn/9JiCtIhuQvbO6fKnv8T4I4?=
 =?us-ascii?Q?0zyMsM6SqFGkPHTVV41E397z2rfw2kyN8/ViXim3PAGaDjFWIQZHiQAzZOlk?=
 =?us-ascii?Q?NZjme5+F/2brOyUOyXEb/0s03o3HBwCF5pZ+FdV8epeLhhm5XSanZJxRR7L5?=
 =?us-ascii?Q?en67Erku6bvwtZMeQAl9Sw9KI+r/8vjR+AyG2OvC4Q8wDZXL3jWPE4Lq2BTG?=
 =?us-ascii?Q?hjKNd2lIlsjMeEo9r7yPVEna/MeN6kY2FnChoYpbmolSvgzbk+qNsnGaKotw?=
 =?us-ascii?Q?5A=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4EAB608EC30A214C8A0DD96C3F510E4B@namprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 9oJaR0xrW+8hkcifMP7Ia6nwF0JZk0zz1nQGya+wDp5a4sYjUGMiyIcMuu9g/pCDGoL3fBXx+Fso8KvjnZYelnGRa97L6s6uXjVUJ+RoCTKj7SozxfooBNEwGMEsjE5XMQpQMFJ2bQKIWQgheLdDk/ZhrLi8Qkut5WjYxZG0f0kN1lBBWgV3YviFPIkmzZSWFjyIFYDt2LiQqiD60h6aIgI88Lb+gnRyGbCGsTUY0x3xfWbqSUTmDivXxgBgXgjqbsdV4ocQwwPyBLPQnWdtegJfwjqYWJ91wxJotjU6mi/OjQVyfb7T9vc8i7OjwneKmXYvG/9MjCydneeISCVj2bWMgVOGB4CLEEttS7w854qm7xIQOgn2pM22SulQjTK+rIGMjOt//4DpvGqflTa9mjxxI41QJbt9UDbZQqTlCAJKdgEFVz+/YAoh6I2EIDO3EhlF+yCarulnp05lm8x0bOyCa//g3NASbrNsukD33t7OGvA7h0RWIw4EphMms2Sf9fqJf1tB3PhEZvMyKKxmRD7CV1i8lHOGFY6pbp59wqkJTqpdAZJn7Z1i3ZPv4fB/DwbyGlErslfB7VEr6LNKvLUR575p/yypfH0F2bOuAvsPTKbPHp/axQN+cnVqAzQxa7vk/QG9bhNt+ifSCyuOrcIWwM7AMpiV/ev/1Q0nwvlIjmoPREc+gHNQ78tuCPIiYLpPMDgzImXR/GYgIrWf0qpC1ua2g9PTdDAHHMhRIOA36zd0PvU9hCQkh6yDQPvEshI+OL6oTFPMGbXsOZgGc+rx3iE1RyinpvQuF4XvGi2hdeusOXmXJyzgeOTfjZcSqW6tcPVO9uDlVJYFihMBPg==
X-OriginatorOrg: wdc.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MN2PR04MB6272.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3c841f23-8ca5-4fdd-85f7-08dbc97037c5
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Oct 2023 09:06:41.2054
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: UdlNCupGr93PLYExR56gL2AYz1FDwc9Qs0mzfgtGhsCW5b/3iT4VfeZ25sTRAw2G2eFqjMRfDr7p3LpLLqeN1g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW6PR04MB9027
X-Spam-Status: No, score=2.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_SBL_CSS,
	SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Tue, 10 Oct 2023 02:07:32 -0700 (PDT)
X-Spam-Level: **

On Tue, Oct 10, 2023 at 07:33:49AM +0900, Damien Le Moal wrote:
> On 10/9/23 21:49, Ma Ke wrote:
> > In mv_platform_probe(), check the return value of clk_prepare_enable()
> > and return the error code if clk_prepare_enable() returns an
> > unexpected value.
> >=20
> > Signed-off-by: Ma Ke <make_ruc2021@163.com>
> > ---
> >  drivers/ata/sata_mv.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >=20
> > diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> > index 45e48d653c60..df3a02e7a50b 100644
> > --- a/drivers/ata/sata_mv.c
> > +++ b/drivers/ata/sata_mv.c
> > @@ -4125,8 +4125,10 @@ static int mv_platform_probe(struct platform_dev=
ice *pdev)
> >  	hpriv->clk =3D clk_get(&pdev->dev, NULL);
> >  	if (IS_ERR(hpriv->clk))
> >  		dev_notice(&pdev->dev, "cannot get optional clkdev\n");
> > -	else
> > -		clk_prepare_enable(hpriv->clk);
> > +	else {
> > +		rc =3D clk_prepare_enable(hpriv->clk);
> > +		goto err;
>=20
> This is wrong. The goto err should only be done if there is an error.
> Did you even test your own patch ???

The "if (rc)" was there in V1....

I should have caught this in my review of V2.

BTW, if we want to be compliant with the coding kernel standard,
you should also have braces on the if (IS_ERR()),
since there are braces on the else-clause.


Kind regards,
Niklas=