Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211030182813.116672-1-hdegoede@redhat.com> <20211030182813.116672-7-hdegoede@redhat.com>
In-Reply-To: <20211030182813.116672-7-hdegoede@redhat.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Sun, 31 Oct 2021 01:07:09 +0300
Message-ID: <CAHp75VeO60umiJTAbL+nR==4pP0KkKQQ71yFNdK2SAkJ0rRXug@mail.gmail.com>
Subject: Re: [PATCH 06/13] power: supply: bq25890: Add support for skipping initialization
To:     Hans de Goede <hdegoede@redhat.com>
Cc:     Mark Gross <markgross@kernel.org>,
        Andy Shevchenko <andy@infradead.org>,
        Wolfram Sang <wsa@the-dreams.de>,
        Sebastian Reichel <sre@kernel.org>,
        MyungJoo Ham <myungjoo.ham@samsung.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        Yauhen Kharuzhy <jekhor@gmail.com>,
        Tsuchiya Yuto <kitakar@gmail.com>,
        Platform Driver <platform-driver-x86@vger.kernel.org>,
        linux-i2c <linux-i2c@vger.kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-efi <linux-efi@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Oct 30, 2021 at 9:28 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> On most X86/ACPI devices there is no devicetree to supply the necessary
> init-data. Instead the firmware already fully initializes the bq25890
> charger at boot.
>
> At support for a new "ti,skip-init" boolean property to support this.
> So far this new property is only used on X86/ACPI (non devicetree) devs,
> IOW it is not used in actual devicetree files. The devicetree-bindings
> maintainers have requested properties like these to not be added to the
> devicetree-bindings, so the new property is deliberately not added
> to the existing devicetree-bindings.

With 'ti,' prefix it can be a potential collision in name space, for
internal properties I would rather use 'linux,' one.

...

> +       init->write_cfg = !device_property_read_bool(bq->dev, "ti,skip-init");
> +       if (!init->write_cfg)
> +               return 0;

Why to have double negation here?
I would rather expect that you will have direct value in the structure
and do a respective check in the functions.

-- 
With Best Regards,
Andy Shevchenko