| 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: Thu, 30 Apr 2015 07:01:32 +0000
From: "Pallala, Ramakrishna" <ramakrishna.pallala@...el.com>
To: Chanwoo Choi <cw00.choi@...sung.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Chanwoo Choi" <cwchoi00@...il.com>,
Lee Jones <lee.jones@...aro.org>,
Samuel Ortiz <sameo@...ux.intel.com>,
Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: RE: [PATCH v8] extcon-axp288: Add axp288 extcon driver support
Hi Choi,
I think I missed some. I will fix them and submit the v9 patch.
> Hi Ram,
>
> I added some comment.
> If you fix minor issue according to comment, I'll apply it.
>
> On 04/30/2015 10:43 AM, Ramakrishna Pallala 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 | 386
> ++++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/axp20x.h | 5 +
> > 4 files changed, 399 insertions(+)
> > create mode 100644 drivers/extcon/extcon-axp288.c
>
> [snip]
>
> > +static int axp288_handle_chrg_det_event(struct axp288_extcon_info
> > +*info) {
> > + static bool notify_otg, notify_charger;
> > + static char *cable;
> > + int ret, stat, cfg, pwr_stat;
> > + u8 chrg_type;
> > + bool vbus_attach = false;
> > +
> > + ret = regmap_read(info->regmap, AXP288_PS_STAT_REG, &pwr_stat);
> > + if (ret < 0) {
> > + dev_err(info->dev, "failed to read vbus status\n");
> > + return ret;
> > + }
> > +
> > + vbus_attach = (pwr_stat & PS_STAT_VBUS_PRESENT);
> > + if (!vbus_attach)
> > + goto notify_otg;
> > +
> > + /* Check charger detection completion status */
> > + ret = regmap_read(info->regmap, AXP288_BC_GLOBAL_REG, &cfg);
> > + if (ret < 0)
> > + goto dev_det_ret;
> > + if (cfg & BC_GLOBAL_DET_STAT) {
> > + dev_dbg(info->dev, "can't complete the charger detection\n");
> > + goto dev_det_ret;
> > + }
> > +
> > + ret = regmap_read(info->regmap, AXP288_BC_DET_STAT_REG, &stat);
> > + if (ret < 0)
> > + goto dev_det_ret;
> > + dev_dbg(info->dev, "status:%x, config:%x\n", stat, cfg);
>
> As I said on previous reply, it is not necessary about just register value without
> appropriate log message.
Ok...
>
> > +
> > + chrg_type = (stat & DET_STAT_MASK) >> DET_STAT_SHIFT;
> > +
> > + switch (chrg_type) {
> > + case DET_STAT_SDP:
> > + dev_dbg(info->dev, "sdp cable is connecetd\n");
> > + notify_otg = true;
> > + notify_charger = true;
> > + cable = AXP288_EXTCON_SLOW_CHARGER;
> > + break;
> > + case DET_STAT_CDP:
> > + dev_dbg(info->dev, "cdp cable is connecetd\n");
> > + notify_otg = true;
> > + notify_charger = true;
> > + cable = AXP288_EXTCON_DOWNSTREAM_CHARGER;
> > + break;
> > + case DET_STAT_DCP:
> > + dev_dbg(info->dev, "dcp cable is connecetd\n");
> > + notify_charger = true;
> > + cable = AXP288_EXTCON_FAST_CHARGER;
> > + break;
> > + default:
> > + dev_warn(info->dev,
> > + "disconnect or unknown or ID event\n");
> > + }
> > +
> > +notify_otg:
> > + if (notify_otg) {
> > + /*
> > + * If VBUS is absent Connect D+/D- lines to PMIC for BC
> > + * detection. Else connect them to SOC for USB communication.
> > + */
> > + if (info->pdata->gpio_mux_cntl)
> > + gpiod_set_value(info->pdata->gpio_mux_cntl,
> > + vbus_attach ? EXTCON_GPIO_MUX_SEL_SOC
> > + :
> EXTCON_GPIO_MUX_SEL_PMIC);
> > +
> > + atomic_notifier_call_chain(&info->otg->notifier,
> > + vbus_attach ? USB_EVENT_VBUS : USB_EVENT_NONE,
> NULL);
> > + }
> > +
> > + if (notify_charger)
> > + extcon_set_cable_state(info->edev, cable, vbus_attach);
> > +
> > + /* Clear the flags on disconnect event */
> > + if (!vbus_attach)
> > + notify_otg = notify_charger = false;
> > +
> > + return 0;
> > +
> > +dev_det_ret:
> > + if (ret < 0)
> > + dev_err(info->dev, "BC Mod detection error\n");
>
> "BC Mod detection error\n" -> "failed to detect BC Mod\n"
>
> > +
> > + return ret;
> > +}
> > +
> > +static irqreturn_t axp288_extcon_isr(int irq, void *data) {
> > + struct axp288_extcon_info *info = data;
> > + int ret;
> > +
> > + ret = axp288_handle_chrg_det_event(info);
> > + if (ret < 0)
> > + dev_err(info->dev, "error in PWRSRC INT handling\n");
>
> As I saied on previous reply, you better to modify log message as following:
> "failed to handle the interrupt\n"
Ok.
>
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void axp288_extcon_enable_irq(struct axp288_extcon_info *info)
> > +{
> > + /* Unmask VBUS interrupt */
> > + regmap_write(info->regmap, AXP288_PWRSRC_IRQ_CFG_REG,
> > + PWRSRC_IRQ_CFG_MASK);
> > + regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
> > + BC_GLOBAL_RUN, 0);
> > + /* Unmask the BC1.2 complete interrupts */
> > + regmap_write(info->regmap, AXP288_BC12_IRQ_CFG_REG,
> BC12_IRQ_CFG_MASK);
> > + /* Enable the charger detection logic */
> > + regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
> > + BC_GLOBAL_RUN, BC_GLOBAL_RUN);
> > +}
> > +
> > +static int axp288_extcon_probe(struct platform_device *pdev) {
>
> [snip]
>
> > +
> > + /* Register extcon device */
> > + ret = devm_extcon_dev_register(&pdev->dev, info->edev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to register extcon device\n");
> > + return ret;
> > + }
> > +
> > + /* Get otg transceiver phy */
> > + info->otg = usb_get_phy(USB_PHY_TYPE_USB2);
> > + if (IS_ERR(info->otg)) {
> > + dev_warn(&pdev->dev, "failed to get otg transceiver\n");
>
> dev_warn -> dev_err
>
> > + return PTR_ERR(info->otg);
> > + }
> > +
> > + /* Set up gpio control for USB Mux */
> > + if (info->pdata->gpio_mux_cntl) {
> > + gpio = desc_to_gpio(info->pdata->gpio_mux_cntl);
> > + ret = gpio_request(gpio, "USB_MUX");
> > + if (ret < 0) {
> > + dev_err(&pdev->dev,
> > + "failed to request the gpio=%d\n", gpio);
> > + goto gpio_req_failed;
> > + }
> > + gpiod_direction_output(info->pdata->gpio_mux_cntl,
> > +
> EXTCON_GPIO_MUX_SEL_PMIC);
> > + }
> > +
> > + for (i = 0; i < EXTCON_IRQ_END; i++) {
> > + pirq = platform_get_irq(pdev, i);
> > + info->irq[i] = regmap_irq_get_virq(info->regmap_irqc, pirq);
> > + if (info->irq[i] < 0) {
> > + dev_warn(&pdev->dev,
> > + "failed to get virtual interrupt=%d\n", pirq);
> > + ret = info->irq[i];
>
> dev_warn -> dev_err
Ok.
Thanks,
Ram
--
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