On Fri, Jul 1, 2022 at 10:35 PM Colin Foster <colin.foster@xxxxxxxxxxxxxxxx> wrote: > On Fri, Jul 01, 2022 at 10:23:36PM +0200, Andy Shevchenko wrote: > > On Fri, Jul 1, 2022 at 9:26 PM Colin Foster > > <colin.foster@xxxxxxxxxxxxxxxx> wrote: ... > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, index); > > > + if (res) { > > > + regs = devm_ioremap_resource(dev, res); > > > + if (IS_ERR(regs)) > > > + return ERR_CAST(regs); > > > > Why can't it be devm_platform_get_and_ioremap_resource() here? > > It can... but it invokes prints of "invalid resource" during > initialization. > > Here it was implied that I should break the function call out: > https://patchwork.kernel.org/project/netdevbpf/patch/20220628081709.829811-2-colin.foster@xxxxxxxxxxxxxxxx/#24917551 Perhaps a comment in the code, so nobody will try to optimize this in the future. > > > + return devm_regmap_init_mmio(dev, regs, config); > > > + } ... > > > + return (map) ? map : ERR_PTR(-ENOENT); > > > > Too many parentheses. > > > > Also you may use short form of ternary operator: > > > > return map ?: ERR_PTR(-ENOENT); > > Agreed, and I didn't know about that operator. When Vladimir suggested > it I thought it was a typo. I should've known better. It's easy to remember by thinking of "X ?: Y" as "X _or_ Y". -- With Best Regards, Andy Shevchenko