| 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
| ||
|
Date: Tue, 20 Oct 2015 09:58:51 -0500 From: "Andrew F. Davis" <afd@...com> To: Lee Jones <lee.jones@...aro.org> CC: 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>, Mark Brown <broonie@...nel.org>, Alexandre Courbot <gnurou@...il.com>, Grygorii Strashko <grygorii.strashko@...com>, <linux-gpio@...r.kernel.org>, <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org> Subject: Re: [PATCH 2/4] mfd: tps65086: Add driver for the TPS65086 PMIC On 10/20/2015 05:02 AM, Lee Jones wrote: > On Mon, 19 Oct 2015, Andrew F. Davis wrote: >> On 10/19/2015 04:23 AM, Lee Jones wrote: >>> On Fri, 16 Oct 2015, Andrew F. Davis wrote: >>> >>>> Add support for the TPS65912 device. It provides communication >>>> through I2C and contains the following components: >>>> >>>> - Regulators >>>> - Load switches >>>> - GPO controller >>> >>> What's a GPO controller? >> >> General Purpose Output, this controller only has output control, it's >> listed in the data-sheet this way, and other drivers have used this term. > > Okay. > >>>> Signed-off-by: Andrew F. Davis <afd@...com> >>>> --- >>>> drivers/mfd/Kconfig | 13 ++++ >>>> drivers/mfd/Makefile | 1 + >>>> drivers/mfd/tps65086.c | 145 +++++++++++++++++++++++++++++++++++++++++++ >>>> include/linux/mfd/tps65086.h | 120 +++++++++++++++++++++++++++++++++++ >>>> 4 files changed, 279 insertions(+) >>>> create mode 100644 drivers/mfd/tps65086.c >>>> create mode 100644 include/linux/mfd/tps65086.h >>>> >>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>>> index 99d6367..ab3b392 100644 >>>> --- a/drivers/mfd/Kconfig >>>> +++ b/drivers/mfd/Kconfig >>>> @@ -1093,6 +1093,19 @@ config TPS6507X >>>> This driver can also be built as a module. If so, the module >>>> will be called tps6507x. >>>> >>>> +config MFD_TPS65086 >>>> + tristate "TI TPS65086x Power Management chips" >>> >>> Use the full name: >>> >>> "TI TPS65086x Power Management Integrated Chips (PMICs)" >>> >>>> + select REGMAP >>>> + select REGMAP_IRQ >>>> + select REGMAP_I2C >>>> + depends on I2C >>> >>> REGMAP_I2C already depends on I2C. >>> >> >> So if I force REGMAP_I2C to =y without depending on I2C then couldn't >> REGMAP_I2C get selected without I2C enabled? Selecting doesn't force >> the selected item's dependencies still right? > > Try it out. :) > OK, I just tried a little test and it did as I expected, I was able to select a symbol and it did not enable its dependency, so this would break build. So, I'm still confused, why does REGMAP_I2C depending on I2C matter? What are you suggesting my Kconfig should look like? >>>> + depends on OF || COMPILE_TEST >>>> + help >>>> + If you say yes here you get support for the TPS65086 series of >>>> + Power Management chips. >>> >>> I'd remove the line break. >>> >>>> + These include voltage regulators, GPO and other features >>> >>> GPIO? >>> >>>> + that are often used in portable devices. >>> >>> This is meant to be a 'help', so instead of staying "and some other >>> stuff", please list them. >>> >> >> This seems to be what other devices do, I'll change it to something >> else then. > > If you know what the "other features" are, it's best to name them. > I was giving a generic description of a PMIC, I didn't really want to list all the features and start marketing the part in the Kconfig, unless you want, I'm sure TI won't mind the advertisement. :) I'm not sure why we write so much about these driver parts at all in the help, people who need them know what they are, it's not like some core kernel config option that people might be undecided on and need explained its function. >>>> config TPS65911_COMPARATOR >>>> tristate >>>> >>>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile >>>> index a59e3fc..7adb902 100644 >>>> --- a/drivers/mfd/Makefile >>>> +++ b/drivers/mfd/Makefile >>>> @@ -66,6 +66,7 @@ obj-$(CONFIG_MFD_WM8994) += wm8994-core.o wm8994-irq.o wm8994-regmap.o >>>> obj-$(CONFIG_TPS6105X) += tps6105x.o >>>> obj-$(CONFIG_TPS65010) += tps65010.o >>>> obj-$(CONFIG_TPS6507X) += tps6507x.o >>>> +obj-$(CONFIG_MFD_TPS65086) += tps65086.o >>>> obj-$(CONFIG_MFD_TPS65217) += tps65217.o >>>> obj-$(CONFIG_MFD_TPS65218) += tps65218.o >>>> obj-$(CONFIG_MFD_TPS65910) += tps65910.o >>>> diff --git a/drivers/mfd/tps65086.c b/drivers/mfd/tps65086.c >>>> new file mode 100644 >>>> index 0000000..be6d678 >>>> --- /dev/null >>>> +++ b/drivers/mfd/tps65086.c >>>> @@ -0,0 +1,145 @@ >>>> +/* >>>> + * Driver for TI TPS65086x PMICs >>> >>> I'd suggest removing the driver part. We know it's a driver. >> >> Again this is what almost every other driver does, > > Almost every other driver. That's some statement. > > I count 42%. ;) > I'm finding in drivers/mfd ~89% of drivers mention at some point that they are a driver. But I do admit my original statement was based on a quick monte carlo selection of a couple files. :) >> I'm not sure how >> else to describe this file without using the word driver. > > Perhaps I'm getting picky in my old age, but I think I'd prefer to see: > > TI TPS65086x PMIC support > Still not sure this describes the file, maybe we could just drop that line completely from files with an obvious purpose? Doesn't really bother me ether way. > [...] > -- 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/
Powered by blists - more mailing lists