| Message ID | 20221118045242.2770-1-cw9316.lee@samsung.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: ufs: ufs-mediatek: Modify the return value | expand |
On Fri, Nov 18, 2022 at 12:57 PM Chanwoo Lee <cw9316.lee@samsung.com> wrote: > > From: ChanWoo Lee <cw9316.lee@samsung.com> > > Change the same as the other code to return bool type. > 91: return !!(host->caps & UFS_MTK_CAP_BOOST_CRYPT_ENGINE); > 98: return !!(host->caps & UFS_MTK_CAP_VA09_PWR_CTRL); > 105: return !!(host->caps & UFS_MTK_CAP_BROKEN_VCC); > > Signed-off-by: ChanWoo Lee <cw9316.lee@samsung.com> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
Il 18/11/22 05:52, Chanwoo Lee ha scritto: > From: ChanWoo Lee <cw9316.lee@samsung.com> > > Change the same as the other code to return bool type. > 91: return !!(host->caps & UFS_MTK_CAP_BOOST_CRYPT_ENGINE); > 98: return !!(host->caps & UFS_MTK_CAP_VA09_PWR_CTRL); > 105: return !!(host->caps & UFS_MTK_CAP_BROKEN_VCC); > > Signed-off-by: ChanWoo Lee <cw9316.lee@samsung.com> > Reviewed-by: Stanley Chu <stanley.chu@mediatek.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
On 11/17/22 20:52, Chanwoo Lee wrote: > From: ChanWoo Lee <cw9316.lee@samsung.com> > > Change the same as the other code to return bool type. > 91: return !!(host->caps & UFS_MTK_CAP_BOOST_CRYPT_ENGINE); > 98: return !!(host->caps & UFS_MTK_CAP_VA09_PWR_CTRL); > 105: return !!(host->caps & UFS_MTK_CAP_BROKEN_VCC); > > Signed-off-by: ChanWoo Lee <cw9316.lee@samsung.com> > --- > drivers/ufs/host/ufs-mediatek.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c > index 7d13878dff47..ef5816d82326 100644 > --- a/drivers/ufs/host/ufs-mediatek.c > +++ b/drivers/ufs/host/ufs-mediatek.c > @@ -109,7 +109,7 @@ static bool ufs_mtk_is_pmc_via_fastauto(struct ufs_hba *hba) > { > struct ufs_mtk_host *host = ufshcd_get_variant(hba); > > - return (host->caps & UFS_MTK_CAP_PMC_VIA_FASTAUTO); > + return !!(host->caps & UFS_MTK_CAP_PMC_VIA_FASTAUTO); > } Hi ChanWoo, Please drop this patch and instead remove the !! from the other functions that have return type 'bool'. There is more Linux kernel code that relies on the implicit conversion from type 'int' to 'bool' than code that converts explicitly from 'int' to 'bool'. Thanks, Bart.
On Fri, 18 Nov 2022, Bart Van Assche wrote:
> There is more Linux kernel code that [...] than code that [...].
Thus mediocrity prevails.
On 11/18/22 13:34, Finn Thain wrote: > On Fri, 18 Nov 2022, Bart Van Assche wrote: > >> There is more Linux kernel code that [...] than code that [...]. > > Thus mediocrity prevails. Mediocrity? I don't understand the above comment. Personally I prefer the style without !! and I don't think that it's a mediocre style. Bart.
On Fri, 18 Nov 2022, Bart Van Assche wrote: > On 11/18/22 13:34, Finn Thain wrote: > > On Fri, 18 Nov 2022, Bart Van Assche wrote: > > > >> There is more Linux kernel code that [...] than code that [...]. > > > > Thus mediocrity prevails. > > Mediocrity? I don't understand the above comment. I'm afraid it was poorly expressed. > Personally I prefer the style without !! and I don't think that it's a > mediocre style. > My comment goes to the rationale you gave not the decision you made. Regarding the decision, it's a choice between "explicit is better than implicit" and "brevity is better than redundancy". The patch opted for the former, you opted for the latter. I also have an opinion, but I'm not the maintainer so I'll keep it to myself. Regarding the rationale, a maintainer who merely follows the majority is not actually doing code review. This will lead to mediocrity.
Chanwoo,
> Change the same as the other code to return bool type.
Applied to 6.2/scsi-staging, thanks!
On Fri, 18 Nov 2022 13:52:42 +0900, Chanwoo Lee wrote: > From: ChanWoo Lee <cw9316.lee@samsung.com> > > Change the same as the other code to return bool type. > 91: return !!(host->caps & UFS_MTK_CAP_BOOST_CRYPT_ENGINE); > 98: return !!(host->caps & UFS_MTK_CAP_VA09_PWR_CTRL); > 105: return !!(host->caps & UFS_MTK_CAP_BROKEN_VCC); > > [...] Applied to 6.2/scsi-queue, thanks! [1/1] scsi: ufs: ufs-mediatek: Modify the return value https://git.kernel.org/mkp/scsi/c/96a2dfa1df4b
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c index 7d13878dff47..ef5816d82326 100644 --- a/drivers/ufs/host/ufs-mediatek.c +++ b/drivers/ufs/host/ufs-mediatek.c @@ -109,7 +109,7 @@ static bool ufs_mtk_is_pmc_via_fastauto(struct ufs_hba *hba) { struct ufs_mtk_host *host = ufshcd_get_variant(hba); - return (host->caps & UFS_MTK_CAP_PMC_VIA_FASTAUTO); + return !!(host->caps & UFS_MTK_CAP_PMC_VIA_FASTAUTO); } static void ufs_mtk_cfg_unipro_cg(struct ufs_hba *hba, bool enable)