Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 13 Jan 2023 23:31:18 +0200
From:   Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To:     Stefan Wahren <stefan.wahren@i2se.com>
Cc:     Bartosz Golaszewski <bartosz.golaszewski@linaro.org>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-rpi-kernel@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@broadcom.com>,
        Ray Jui <rjui@broadcom.com>,
        Scott Branden <sbranden@broadcom.com>
Subject: Re: [PATCH v2 1/2] pinctrl: bcm: bcm2835: Switch to use
 ->add_pin_ranges()
Message-ID: <Y8HNpokX5qCQTRqu@smile.fi.intel.com>
References: <20230113171051.19309-1-andriy.shevchenko@linux.intel.com>
 <916654ca-e70f-5663-f3a3-9b370c24aea9@i2se.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <916654ca-e70f-5663-f3a3-9b370c24aea9@i2se.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Fri, Jan 13, 2023 at 09:13:23PM +0100, Stefan Wahren wrote:
> Am 13.01.23 um 18:10 schrieb Andy Shevchenko:

...

> > v2: fixed compilation issues (LKP), Cc'ed to the author of original code
> > 
> > Btw, the commit d2b67744fd99 ("pinctrl: bcm2835: implement hook for
> > missing gpio-ranges") seems problematic in the fist place due to
> > odd of_node_put() call. I dunno how that part had been tested, or
> > how it's supposed to work, i.e. where is the counterpart of_node_get().
> > Anyway this change drops it for good.
> 
> The countpart is in of_pinctrl_get(). I was just following the pattern like
> in other drivers like gpio-rockchip. The original commit has been tested by
> Florian Fainelli and me. I'm not sure if it's safe to drop it completely.

Please, elaborate how of_pinctrl_get() increases refcount of the parameter.
Maybe I'm looking into a wrong place?

> Btw this is not the only platform affected by the gpio-ranges compatibility
> issue [1].

This is the only one that uses unnecessary added callback.

> > Perhaps we can check gpio-ranges property presence inside the GPIO
> > library, so this ->add_pin_ranges() won't be called at all.
> 
> I thought this could be very platform specific, so i implemented a hook. But
> yes my initial hack modified gpiolib-of [2].

The point is that possibly documentation of ->add_pin_ranges() should be
amended to take care of the cases like this. We don't need two or more
hooks to do the same, esp. taking into account that OF specific doesn't
cover other cases.

> [1] -?https://patchwork.kernel.org/project/linux-arm-msm/patch/20180412190138.12372-1-chunkeey@gmail.com/
> 
> [2] - https://lore.kernel.org/linux-arm-kernel/75266ed1-666a-138b-80f1-ae9a06b7bdf3@i2se.com/
> 
> > Also I would like to understand the dance around checking for pin
> > control device. The original commit lacks of comments in the non-trivial
> > code.

Any comment on this?

-- 
With Best Regards,
Andy Shevchenko