| Message ID | 20211224131300.18198-3-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,01/10] ata: pata_platform: Make use of platform_get_mem_or_io() | expand |
On 12/24/21 4:12 PM, Lad Prabhakar wrote: > pata_platform_probe() isn't a hotpath, which makes it's questionable to > use unlikely(). Therefore let's simply drop it. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > v2-->v3 > * New patch > --- > drivers/ata/pata_platform.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c > index cb3134bf88eb..29902001e223 100644 > --- a/drivers/ata/pata_platform.c > +++ b/drivers/ata/pata_platform.c > @@ -199,14 +199,14 @@ static int pata_platform_probe(struct platform_device *pdev) > * Get the I/O base first > */ > io_res = platform_get_mem_or_io(pdev, 0); > - if (unlikely(!io_res)) > + if (!io_res) > return -EINVAL; > > /* > * Then the CTL base > */ > ctl_res = platform_get_mem_or_io(pdev, 1); > - if (unlikely(!ctl_res)) > + if (!ctl_res) > return -EINVAL; I think you should combine this with patch #1. [...] MBR, Sergey
Hi Sergey, Thank you for the review. On Fri, Dec 24, 2021 at 5:54 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > > On 12/24/21 4:12 PM, Lad Prabhakar wrote: > > > pata_platform_probe() isn't a hotpath, which makes it's questionable to > > use unlikely(). Therefore let's simply drop it. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > v2-->v3 > > * New patch > > --- > > drivers/ata/pata_platform.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c > > index cb3134bf88eb..29902001e223 100644 > > --- a/drivers/ata/pata_platform.c > > +++ b/drivers/ata/pata_platform.c > > @@ -199,14 +199,14 @@ static int pata_platform_probe(struct platform_device *pdev) > > * Get the I/O base first > > */ > > io_res = platform_get_mem_or_io(pdev, 0); > > - if (unlikely(!io_res)) > > + if (!io_res) > > return -EINVAL; > > > > /* > > * Then the CTL base > > */ > > ctl_res = platform_get_mem_or_io(pdev, 1); > > - if (unlikely(!ctl_res)) > > + if (!ctl_res) > > return -EINVAL; > > I think you should combine this with patch #1. > I'd like to keep the changes separate from patch #1, as it's unrelated. Cheers, Prabhakar
On 12/25/21 03:02, Lad, Prabhakar wrote: > Hi Sergey, > > Thank you for the review. > > On Fri, Dec 24, 2021 at 5:54 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: >> >> On 12/24/21 4:12 PM, Lad Prabhakar wrote: >> >>> pata_platform_probe() isn't a hotpath, which makes it's questionable to >>> use unlikely(). Therefore let's simply drop it. >>> >>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >>> --- >>> v2-->v3 >>> * New patch >>> --- >>> drivers/ata/pata_platform.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c >>> index cb3134bf88eb..29902001e223 100644 >>> --- a/drivers/ata/pata_platform.c >>> +++ b/drivers/ata/pata_platform.c >>> @@ -199,14 +199,14 @@ static int pata_platform_probe(struct platform_device *pdev) >>> * Get the I/O base first >>> */ >>> io_res = platform_get_mem_or_io(pdev, 0); >>> - if (unlikely(!io_res)) >>> + if (!io_res) >>> return -EINVAL; >>> >>> /* >>> * Then the CTL base >>> */ >>> ctl_res = platform_get_mem_or_io(pdev, 1); >>> - if (unlikely(!ctl_res)) >>> + if (!ctl_res) >>> return -EINVAL; >> >> I think you should combine this with patch #1. >> > I'd like to keep the changes separate from patch #1, as it's unrelated. But your patch 1 adds the unlikely... So simply do not add it in patch one and this patch is not necessary anymore. > > Cheers, > Prabhakar
Hi Damien, On Sun, Dec 26, 2021 at 11:56 AM Damien Le Moal <damien.lemoal@opensource.wdc.com> wrote: > > On 12/25/21 03:02, Lad, Prabhakar wrote: > > Hi Sergey, > > > > Thank you for the review. > > > > On Fri, Dec 24, 2021 at 5:54 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > >> > >> On 12/24/21 4:12 PM, Lad Prabhakar wrote: > >> > >>> pata_platform_probe() isn't a hotpath, which makes it's questionable to > >>> use unlikely(). Therefore let's simply drop it. > >>> > >>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > >>> --- > >>> v2-->v3 > >>> * New patch > >>> --- > >>> drivers/ata/pata_platform.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c > >>> index cb3134bf88eb..29902001e223 100644 > >>> --- a/drivers/ata/pata_platform.c > >>> +++ b/drivers/ata/pata_platform.c > >>> @@ -199,14 +199,14 @@ static int pata_platform_probe(struct platform_device *pdev) > >>> * Get the I/O base first > >>> */ > >>> io_res = platform_get_mem_or_io(pdev, 0); > >>> - if (unlikely(!io_res)) > >>> + if (!io_res) > >>> return -EINVAL; > >>> > >>> /* > >>> * Then the CTL base > >>> */ > >>> ctl_res = platform_get_mem_or_io(pdev, 1); > >>> - if (unlikely(!ctl_res)) > >>> + if (!ctl_res) > >>> return -EINVAL; > >> > >> I think you should combine this with patch #1. > >> > > I'd like to keep the changes separate from patch #1, as it's unrelated. > > But your patch 1 adds the unlikely... So simply do not add it in patch > one and this patch is not necessary anymore. > patch #1 just replaces two platform_get_resource() with one platform_get_mem_or_io() call, the unlikely() is just indented towards the left. But anyway I can merge this into #1. Are you OK with the rest of the patches? Cheers, Prabhakar
diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c index cb3134bf88eb..29902001e223 100644 --- a/drivers/ata/pata_platform.c +++ b/drivers/ata/pata_platform.c @@ -199,14 +199,14 @@ static int pata_platform_probe(struct platform_device *pdev) * Get the I/O base first */ io_res = platform_get_mem_or_io(pdev, 0); - if (unlikely(!io_res)) + if (!io_res) return -EINVAL; /* * Then the CTL base */ ctl_res = platform_get_mem_or_io(pdev, 1); - if (unlikely(!ctl_res)) + if (!ctl_res) return -EINVAL; /*
pata_platform_probe() isn't a hotpath, which makes it's questionable to use unlikely(). Therefore let's simply drop it. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- v2-->v3 * New patch --- drivers/ata/pata_platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)