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: <CAGTfZH0mK1Cqx=JVDRy3HerOtNv=GEXCMSjXWjyLZhiBhinyAw@mail.gmail.com>
Date:	Tue, 28 Apr 2015 20:33:22 +0900
From:	Chanwoo Choi <cwchoi00@...il.com>
To:	"Pallala, Ramakrishna" <ramakrishna.pallala@...el.com>
Cc:	Lee Jones <lee.jones@...aro.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: Re: [PATCH v5] extcon-axp288: Add axp288 extcon driver support

Hi Ram,

On Tue, Apr 28, 2015 at 8:20 PM, Pallala, Ramakrishna
<ramakrishna.pallala@...el.com> wrote:
>> On Tue, 28 Apr 2015, Pallala, Ramakrishna wrote:
>>
>> > > > This patch adds the extcon support for AXP288 PMIC which has the
>> > > > BC1.2 charger detection capability. Additionally it also adds the
>> > > > USB mux switching support b/w SOC and PMIC based on GPIO control.
>> > > >
>> > > > Signed-off-by: Ramakrishna Pallala <ramakrishna.pallala@...el.com>
>> > > > ---
>> > > >  drivers/extcon/Kconfig         |    7 +
>> > > >  drivers/extcon/Makefile        |    1 +
>> > > >  drivers/extcon/extcon-axp288.c |  399
>> > > ++++++++++++++++++++++++++++++++++++++++
>> > > >  include/linux/mfd/axp20x.h     |    9 +
>> > > >  4 files changed, 416 insertions(+)  create mode 100644
>> > > > drivers/extcon/extcon-axp288.c
>> > > >
>> > > > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig index
>> > > > fdc0bf0..2305edc 100644
>> > > > --- a/drivers/extcon/Kconfig
>> > > > +++ b/drivers/extcon/Kconfig
>> > > > @@ -28,6 +28,13 @@ config EXTCON_ARIZONA
>> > > >           with Wolfson Arizona devices. These are audio CODECs with
>> > > >           advanced audio accessory detection support.
>> > > >
>> > > > +config EXTCON_AXP288
>> > > > +       tristate "X-Power AXP288 EXTCON support"
>> > > > +       depends on MFD_AXP20X && USB_PHY
>> > > > +       help
>> > > > +         Say Y here to enable support for USB peripheral detection
>> > > > +         and USB MUX switching by X-Power AXP288 PMIC.
>> > > > +
>> > > >  config EXTCON_GPIO
>> > > >         tristate "GPIO extcon support"
>> > > >         depends on GPIOLIB
>> > > > diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
>> > > > index
>> > > > 9204114..ba787d0 100644
>> > > > --- a/drivers/extcon/Makefile
>> > > > +++ b/drivers/extcon/Makefile
>> > > > @@ -5,6 +5,7 @@
>> > > >  obj-$(CONFIG_EXTCON)           += extcon.o
>> > > >  obj-$(CONFIG_EXTCON_ADC_JACK)  += extcon-adc-jack.o
>> > > >  obj-$(CONFIG_EXTCON_ARIZONA)   += extcon-arizona.o
>> > > > +obj-$(CONFIG_EXTCON_AXP288)    += extcon-axp288.o
>> > > >  obj-$(CONFIG_EXTCON_GPIO)      += extcon-gpio.o
>> > > >  obj-$(CONFIG_EXTCON_MAX14577)  += extcon-max14577.o
>> > > >  obj-$(CONFIG_EXTCON_MAX77693)  += extcon-max77693.o
>> > > > diff --git a/drivers/extcon/extcon-axp288.c
>> > > > b/drivers/extcon/extcon-axp288.c new file mode 100644 index
>> > > > 0000000..91d764c
>> > > > --- /dev/null
>> > > > +++ b/drivers/extcon/extcon-axp288.c
>> > > > @@ -0,0 +1,399 @@
>> > > > +/*
>> > > > + * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection
>> > > > +driver
>> > > > + *
>> > > > + * Copyright (C) 2014 Intel Corporation
>> > > > + * Author: Ramakrishna Pallala <ramakrishna.pallala@...el.com>
>> > > > + *
>> > > > + * This program is free software; you can redistribute it and/or
>> > > > +modify
>> > > > + * it under the terms of the GNU General Public License version 2
>> > > > +as
>> > > > + * published by the Free Software Foundation.
>> > > > + *
>> > > > + * This program is distributed in the hope that it will be
>> > > > +useful,
>> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > > > + * GNU General Public License for more details.
>> > > > + */
>> > > > +
>> > > > +#include <linux/module.h>
>> > > > +#include <linux/kernel.h>
>> > > > +#include <linux/io.h>
>> > > > +#include <linux/slab.h>
>> > > > +#include <linux/interrupt.h>
>> > > > +#include <linux/platform_device.h> #include <linux/property.h>
>> > > > +#include <linux/usb/phy.h> #include <linux/notifier.h> #include
>> > > > +<linux/extcon.h> #include <linux/regmap.h> #include
>> > > > +<linux/gpio.h> #include <linux/gpio/consumer.h> #include
>> > > > +<linux/mfd/axp20x.h>
>> >
>> > [snip]
>> >
>> > > > +module_platform_driver(axp288_extcon_driver);
>> > > > +
>> > > > +MODULE_AUTHOR("Ramakrishna Pallala
>> > > > +<ramakrishna.pallala@...el.com>");
>> > > > +MODULE_DESCRIPTION("X-Powers AXP288 extcon driver");
>> > > > +MODULE_LICENSE("GPL v2");
>> > > > diff --git a/include/linux/mfd/axp20x.h
>> > > > b/include/linux/mfd/axp20x.h index dfabd6d..81152e2 100644
>> > > > --- a/include/linux/mfd/axp20x.h
>> > > > +++ b/include/linux/mfd/axp20x.h
>> > > > @@ -275,4 +275,13 @@ struct axp20x_fg_pdata {
>> > > >         int thermistor_curve[MAX_THERM_CURVE_SIZE][2];
>> > > >  };
>> > > >
>> > > > +#define AXP288_EXTCON_CABLE_SDP                "SLOW-CHARGER"
>> > > > +#define AXP288_EXTCON_CABLE_CDP                "CHARGE-
>> > > DOWNSTREAM"
>> > > > +#define AXP288_EXTCON_CABLE_DCP                "FAST-CHARGER"
>> > >
>> > > #defines are meant to make things *easier* for humans to read. It is
>> > > my opinion that these negatively impact the readability of the code.
>> > These macros will be used in extcon and charger drivers. How can I make it
>> more readable?
>>
>> If the things you're defining are more readable than the defines themselves
>> you're doing the opposite of what I'd expect.
>>
>> How about:
>>
>>   AXP288_EXTCON_SLOW_CHARGER_CABLE
>>   AXP288_EXTCON_FAST_CHARGER_CABLE
>>   AXP288_EXTCON_CHARGE_DOWNSTREAM_CABLE
>
> Ok I will change the names similar to the above ones.

This cable names are used in only extcon driver.
I think that you don't need to define them on header file.
You better to move this definition to extcon driver.

Also, I'll review your patch on other mail.

Thanks,
Chanwoo Choi
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ