From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751172AbbD3Gxv (ORCPT ); Thu, 30 Apr 2015 02:53:51 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:14067 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750768AbbD3Gxt (ORCPT ); Thu, 30 Apr 2015 02:53:49 -0400 X-AuditID: cbfee691-f79ca6d00000456a-dc-5541d17aabd3 Message-id: <5541D17A.4010903@samsung.com> Date: Thu, 30 Apr 2015 15:53:46 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-version: 1.0 To: Ramakrishna Pallala Cc: linux-kernel@vger.kernel.org, Chanwoo Choi , Lee Jones , Samuel Ortiz , Jacob Pan Subject: Re: [PATCH v8] extcon-axp288: Add axp288 extcon driver support References: <1430358217-11191-1-git-send-email-ramakrishna.pallala@intel.com> In-reply-to: <1430358217-11191-1-git-send-email-ramakrishna.pallala@intel.com> Content-type: text/plain; charset=ISO-8859-1 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprNIsWRmVeSWpSXmKPExsWyRsSkQLfqomOowbkmK4tnR7Utlh5cz2Jx /+tRRovLu+awWSx8c5PJ4nQ3qwObx85Zd9k9Fu95yeRx59oeNo95JwM9Pm+SC2CN4rJJSc3J LEst0rdL4Mr417aYpWCfbsXN+d3MDYxflLsYOTkkBEwkjnzfxAphi0lcuLeerYuRi0NIYCmj xJVjs9lginZOPM4MkVjEKPF/0WtGCOcBo8TLkx/YQap4BbQk5i3bzQRiswioSrzcOhdsLBtQ fP+LG2CTRAXCJFZOv8ICUS8o8WPyPSCbg0NEwFzi3n8VkJnMArsYJdbtXs8MUiMs4C7R/Pkj O0iNkICvxKENviBhTgE/ibmXXoGVMAvoSOxvncYGYctLbF7zFuxQCYFj7BLtPxdA3SMg8W3y IbBdEgKyEpsOMEM8JilxcMUNlgmMYrOQXDQLydhZSMYuYGRexSiaWpBcUJyUXmSqV5yYW1ya l66XnJ+7iREYZ6f/PZu4g/H+AetDjAIcjEo8vB/aHUOFWBPLiitzDzGaAl0xkVlKNDkfGM15 JfGGxmZGFqYmpsZG5pZmSuK8OtI/g4UE0hNLUrNTUwtSi+KLSnNSiw8xMnFwSjUwnvSP/W+W MCkgs6/Ko/zIjUMrJormFrOVmK+X37T4XOKhvIq26E4rmUKu2/2RoVNNxZ9lRLmo7As1UuiR sF0su9/y2Wq1CUl+ihs5XK6+9VTzi8sSPrZVq7XHVmWnSsQG3jqH+uez/2+KkmLaf/jzJYuG 1vTVk66+fX2+/cTO6994Fkp3y2QosRRnJBpqMRcVJwIAqKEwWa4CAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrKIsWRmVeSWpSXmKPExsVy+t9jAd2qi46hBjf3m1g8O6ptsfTgehaL +1+PMlpc3jWHzWLhm5tMFqe7WR3YPHbOusvusXjPSyaPO9f2sHnMOxno8XmTXABrVAOjTUZq YkpqkUJqXnJ+SmZeuq2Sd3C8c7ypmYGhrqGlhbmSQl5ibqqtkotPgK5bZg7QfiWFssScUqBQ QGJxsZK+HaYJoSFuuhYwjRG6viFBcD1GBmggYQ1jxr+2xSwF+3Qrbs7vZm5g/KLcxcjJISFg IrFz4nFmCFtM4sK99WxdjFwcQgKLGCX+L3rNCOE8YJR4efIDO0gVr4CWxLxlu5lAbBYBVYmX W+eygthsQPH9L26wgdiiAmESK6dfYYGoF5T4MfkekM3BISJgLnHvvwrITGaBXYwS63avB9ss LOAu0fz5IztIjZCAr8ShDb4gYU4BP4m5l16BlTAL6Ejsb53GBmHLS2xe85Z5AqPALCQbZiEp m4WkbAEj8ypG0dSC5ILipPRcQ73ixNzi0rx0veT83E2M4Ch+JrWDcWWDxSFGAQ5GJR7eD+2O oUKsiWXFlbmHGCU4mJVEeCXWA4V4UxIrq1KL8uOLSnNSiw8xmgIDYCKzlGhyPjDB5JXEGxqb mBlZGpkbWhgZmyuJ887RlQsVEkhPLEnNTk0tSC2C6WPi4JRqYHQL/H6uhoXvw6Kpm/gnGwVN 07pzJZbvWElkM9tBf67dLgkXrx1Z+XKu4n6PB+ed37C7azUX5anJWDA38Pq1fNO/dpD7SuVb n+1BSbdvX1vLPTc4MPRYX1626L83a378Djl7mfNIzN6VplcYn83Tj56xeNO7d/O+yav2PK/t F38rxfdVMnGq7VclluKMREMt5qLiRADPxy+H+AIAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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. > + > + 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" > + > + 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 [snip] Thanks, Chanwoo Choi