linux-kernel - Re: [PATCH v4 2/6] mfd: axp20x: Split the driver into core and i2c bits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 24 Nov 2015 17:21:31 +0200
From:	Andy Shevchenko <andy.shevchenko@...il.com>
To:	Chen-Yu Tsai <wens@...e.org>
Cc:	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Lee Jones <lee.jones@...aro.org>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	devicetree <devicetree@...r.kernel.org>,
	linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-sunxi <linux-sunxi@...glegroups.com>,
	Hans de Goede <hdegoede@...hat.com>,
	Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH v4 2/6] mfd: axp20x: Split the driver into core and i2c bits

On Tue, Nov 24, 2015 at 5:09 PM, Chen-Yu Tsai <wens@...e.org> wrote:
> On Tue, Nov 24, 2015 at 8:35 PM, Andy Shevchenko
> <andy.shevchenko@...il.com> wrote:
>> On Tue, Nov 24, 2015 at 1:28 PM, Chen-Yu Tsai <wens@...e.org> wrote:
>>> On Tue, Nov 24, 2015 at 5:37 PM, Andy Shevchenko
>>> <andy.shevchenko@...il.com> wrote:
>>>> On Tue, Nov 24, 2015 at 5:48 AM, Chen-Yu Tsai <wens@...e.org> wrote:
>>>>> The axp20x driver assumes the device is i2c based. This is not the
>>>>> case with later chips, which use a proprietary 2 wire serial bus
>>>>> by Allwinner called "Reduced Serial Bus".
>>>>>
>>>>> This patch follows the example of mfd/wm831x and splits it into
>>>>> an interface independent core, and an i2c specific glue layer.
>>>>> MFD_AXP20X and the new MFD_AXP20X_I2C are changed to tristate
>>>>> symbols, allowing the driver to be built as modules.
>>>>>
>>>>> Included but unused header files are removed as well.
>>
>> So…
>>
>>>>> +       if (dev->of_node) {
>>>>
>>>> What about
>>>>
>>>> if (…of_node) {
>>>>       const struct of_device_id *id;
>>>> …
>>>> } else if ACPI_COMPANION(…) {
>>
>> This should be has_acpi_companion().
>
> I don't think the "else if" is necessary. There's only 2 possible ways
> the device gets probed, either device tree or ACPI.

OK.

It would be ideal to move to unified device properties API at some
point, but it's out of scope of this series anyway.

>
>>>>       const struct acpi_device_id *id;
>>>> …
>>>> } else {
>>>>  return -ENODEV;
>>>> }
>>>
>>> I really don't want to change code that I'm just moving around.
>>> Same goes for the other comments about this patch. I can do another
>>> patch on top of this to fix the style issues if it really bothers
>>> people.
>>
>> Fair enough.
>> My comments mostly about unnecessity of second parameter in the functions.
>>
>> So,  you already did some clean up in this patch (above), what about
>> to do another? I also prefer separate patch *before* you do a split.
>
> Sure. I'll do a patch or 2 before the split. Would you mind if I add your
> Suggested-by tag?

I would not.

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/