Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753646Ab1CPSzY (ORCPT ); Wed, 16 Mar 2011 14:55:24 -0400 Received: from wolverine02.qualcomm.com ([199.106.114.251]:17905 "EHLO wolverine02.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752106Ab1CPSzU (ORCPT ); Wed, 16 Mar 2011 14:55:20 -0400 X-IronPort-AV: E=McAfee;i="5400,1158,6287"; a="80144494" Message-ID: <4D810797.4090000@codeaurora.org> Date: Wed, 16 Mar 2011 11:55:19 -0700 From: Abhijeet Dharmapurikar User-Agent: Thunderbird 2.0.0.22 (X11/20090608) MIME-Version: 1.0 To: Grant Likely CC: davidb@codeaurora.org, "David S. Miller" , Andrew Morton , Bryan Huntsman , Daniel Walker , David Collins , Greg Kroah-Hartman , Joe Perches , Russell King , Samuel Ortiz , Stepan Moskovchenko , Mark Brown , Linus Walleij , Thomas Glexiner , linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [Qualcomm PM8921 MFD v2 3/6] gpio: pm8xxx-gpio: Add pm8xxx gpio driver References: <1299564590-30116-1-git-send-email-adharmap@codeaurora.org> <1299564590-30116-4-git-send-email-adharmap@codeaurora.org> <20110312093645.GL9347@angua.secretlab.ca> In-Reply-To: <20110312093645.GL9347@angua.secretlab.ca> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3080 Lines: 91 >> >> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig >> index 664660e..c5e6f51 100644 >> --- a/drivers/gpio/Kconfig >> +++ b/drivers/gpio/Kconfig >> @@ -411,4 +411,14 @@ config GPIO_JANZ_TTL >> This driver provides support for driving the pins in output >> mode only. Input mode is not supported. >> >> +comment "SSBI GPIO expanders:" > > SSBI? Also, the comment seems rather out of place when there > currently appears to only be one of such devices. SSBI is a bus architecture used to access this device's register (actually this is a subdevice in the pmic and ssbi is used to access all the registers in the pmic).The bus driver can be found here https://patchwork.kernel.org/patch/601771/ It didnt occur to me that the comment is only meant for buses which have more than one gpio devices. I saw comment "MODULbus GPIO expanders:" comment "AC97 GPIO expanders:" both containing single devices and thought it is a norm to add a comment if a device running on a new bus is introduced. Let me know if you still think I should remove comment "SSBI GPIO expanders:" ? >> + >> +struct pm_gpio_chip { >> + struct list_head link; >> + struct gpio_chip gpio_chip; >> + struct mutex pm_lock; >> + u8 *bank1; >> + int irq_base; >> +}; >> + >> +static LIST_HEAD(pm_gpio_chips); > > Looks like you need a mutex for protecting this list from mutual access. Yes will fix this. > >> + >> +#ifndef __PM8XXX_GPIO_H >> +#define __PM8XXX_GPIO_H >> + >> +#include >> + >> +#define PM8XXX_GPIO_DEV_NAME "pm8xxx-gpio" >> + >> +struct pm8xxx_gpio_core_data { >> + u32 rev; >> + int ngpios; >> +}; >> + >> +struct pm8xxx_gpio_platform_data { >> + struct pm8xxx_gpio_core_data gpio_cdata; >> + int gpio_base; >> +}; > > There doesn't seem to be any value it splitting pm8xxx_gpio_core_data > into a separate structure from what I see in this patch. How is this > going to be used? gpio_base comes from the platform data because the board file knows where in the global gpio numbers the pm8xxx gpio start. For example if the MSM code supports 150 gpios(0 through 149), the board file will set gpio_base to indicate pm8xxx gpios should start from 150. The pm8xxx_gpio_core_data is meant to be filled in by the pm8921 core. We have different pmic chips with similar gpio implementations. For example 8058 pmic, 8901 pmic and 8921 pmic, all have the same gpio implementation but different number of gpio lines. The pm8xxx-gpio.c is used for all these pmics. Hence we separated core specific gpio information (number of gpio lines supported) from board specific gpio information (where in the global map the gpio number for this device starts). -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/