linux-kernel - Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <21684e2a-e84c-05ed-e27c-e710c53e3a64@lucaceresoli.net>
Date:   Wed, 13 Oct 2021 23:49:40 +0200
From:   Luca Ceresoli <luca@...aceresoli.net>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        linux-kernel@...r.kernel.org
Cc:     Lee Jones <lee.jones@...aro.org>, Rob Herring <robh+dt@...nel.org>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Guenter Roeck <linux@...ck-us.net>, devicetree@...r.kernel.org,
        linux-rtc@...r.kernel.org, linux-watchdog@...r.kernel.org,
        Chiwoong Byun <woong.byun@...sung.com>,
        Laxman Dewangan <ldewangan@...dia.com>
Subject: Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC

Hi,

On 12/10/21 10:09, Krzysztof Kozlowski wrote:
> On 11/10/2021 17:56, Luca Ceresoli wrote:
>> Add a simple driver for the Maxim MAX77714 PMIC, supporting RTC and
>> watchdog only.
>>
>> Signed-off-by: Luca Ceresoli <luca@...aceresoli.net>
>> ---
>>  MAINTAINERS                  |   2 +
>>  drivers/mfd/Kconfig          |  14 ++++
>>  drivers/mfd/Makefile         |   1 +
>>  drivers/mfd/max77714.c       | 151 +++++++++++++++++++++++++++++++++++
>>  include/linux/mfd/max77714.h |  68 ++++++++++++++++
>>  5 files changed, 236 insertions(+)
>>  create mode 100644 drivers/mfd/max77714.c
>>  create mode 100644 include/linux/mfd/max77714.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 4d0134752537..df394192f14e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11389,6 +11389,8 @@ MAXIM MAX77714 PMIC MFD DRIVER
>>  M:	Luca Ceresoli <luca@...aceresoli.net>
>>  S:	Maintained
>>  F:	Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>> +F:	drivers/mfd/max77714.c
>> +F:	include/linux/mfd/max77714.h
>>  
>>  MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
>>  M:	Javier Martinez Canillas <javier@...hile0.org>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index ca0edab91aeb..b5f6e6174508 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -853,6 +853,20 @@ config MFD_MAX77693
>>  	  additional drivers must be enabled in order to use the functionality
>>  	  of the device.
>>  
>> +config MFD_MAX77714
>> +	bool "Maxim Semiconductor MAX77714 PMIC Support"
> 
> Why it cannot be a tristate (module)?

Because it's not done in the driver I initially copied from, I guess. :)

And also because I thought it's appropriate for a PMIC driver since
regulators tend to be always instantiated. But I understand there are
valid use cases for that -- will do in v2 unless a good reason pops up
for not doing it.

>> diff --git a/include/linux/mfd/max77714.h b/include/linux/mfd/max77714.h
>> new file mode 100644
>> index 000000000000..ca6b747b73c2
>> --- /dev/null
>> +++ b/include/linux/mfd/max77714.h
>> @@ -0,0 +1,68 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Maxim MAX77714 Register and data structures definition.
>> + *
>> + * Copyright (C) 2021 Luca Ceresoli
>> + * Author: Luca Ceresoli <luca@...aceresoli.net>
>> + */
>> +
>> +#ifndef _MFD_MAX77714_H_
>> +#define _MFD_MAX77714_H_
> 
> Header guard:
> __LINUX_MFD_MAX77714_H_

OK.

>> +
>> +struct max77714 {
>> +	struct device *dev;
>> +	struct regmap *regmap;
>> +	struct regmap_irq_chip_data *irq_data;
>> +
>> +	int irq;
>> +};
> 
> Do you have to make it a public structure? If not, please put it in the
> max77714.c

Good point. Will fix.

-- 
Luca