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;
Message-ID: <10c9d774-dcae-e80d-e6fa-235410658b84@redhat.com>
Date:   Mon, 8 Nov 2021 16:57:44 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: [PATCH 06/13] power: supply: bq25890: Add support for skipping
 initialization
Content-Language: en-US
To:     Andy Shevchenko <andy.shevchenko@gmail.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>
References: <20211030182813.116672-1-hdegoede@redhat.com>
 <20211030182813.116672-7-hdegoede@redhat.com>
 <CAHp75VeO60umiJTAbL+nR==4pP0KkKQQ71yFNdK2SAkJ0rRXug@mail.gmail.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <CAHp75VeO60umiJTAbL+nR==4pP0KkKQQ71yFNdK2SAkJ0rRXug@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 10/31/21 00:07, Andy Shevchenko wrote:
> 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.

Good point, changed for v2.

> ...
> 
>> +       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.

Because in all places except this one we want to know if we need to
write the cfg to the device, removing the double negation here would
mean adding negation to a init->skip_init check in many places, so this
is cleaner.

Regards,

Hans