All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trilok Soni <tsoni@codeaurora.org>
To: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>,
	davidb@codeaurora.org, "David S. Miller" <davem@davemloft.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	Bryan Huntsman <bryanh@codeaurora.org>,
	Daniel Walker <dwalker@fifo99.com>,
	David Collins <collinsd@codeaurora.org>,
	Grant Likely <grant.likely@secretlab.ca>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	Joe Perches <joe@perches.com>,
	Russell King <linux@arm.linux.org.uk>,
	Samuel Ortiz <sameo@linux.intel.com>,
	Stepan Moskovchenko <stepanm@codeaurora.org>,
	Linus Walleij <linux.walleij@sterricsson.com>,
	Thomas Glexiner <tglx@linutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [Qualcomm PM8921 MFD v2 4/6] mfd: pm8xxx-mpp: Add pm8xxx MPP driver
Date: Thu, 10 Mar 2011 09:26:38 +0530	[thread overview]
Message-ID: <4D784BF6.1040700@codeaurora.org> (raw)
In-Reply-To: <4D784730.4010307@codeaurora.org>

Hi Abhi,

On 3/10/2011 9:06 AM, Abhijeet Dharmapurikar wrote:
> Mark Brown wrote:
>> On Mon, Mar 07, 2011 at 10:09:48PM -0800, adharmap@codeaurora.org wrote:
>>
>>> +    mpp_chip->gpio_chip.label = PM8XXX_MPP_DEV_NAME;
>>> +    mpp_chip->gpio_chip.direction_input = pm8xxx_mpp_dir_input;
>>> +    mpp_chip->gpio_chip.direction_output = pm8xxx_mpp_dir_output;
>>> +    mpp_chip->gpio_chip.to_irq = pm8xxx_mpp_to_irq;
>>> +    mpp_chip->gpio_chip.get = pm8xxx_mpp_get;
>>> +    mpp_chip->gpio_chip.set = pm8xxx_mpp_set;
>>> +    mpp_chip->gpio_chip.dbg_show = pm8xxx_mpp_dbg_show;
>>> +    mpp_chip->gpio_chip.ngpio = pdata->core_data.nmpps;
>>> +    mpp_chip->gpio_chip.can_sleep = 1;
>>> +    mpp_chip->gpio_chip.dev = &pdev->dev;
>>> +    mpp_chip->gpio_chip.base = pdata->mpp_base;
>>
>> It's seems really odd that you're adding gpiolib stuff here when you've
>> also got a separate gpiolib driver.  Possibly this all shouldn't
>> actually be split up as much as it is - there's also the issue with the
>> gpiolib driver needing to peer into the interrupt controller.  It might
>> simplify the code if things were merged more.
> 
> I dont think merging code will help here. gpio lines and mpp lines are very different piece of hardware,they have a different register map and different config attributes. They do fall under the generic 'gpiolib' umbrella, but it seems clean to keep them separate.
> 
> I agree with you that gpio code needs to call on interrupt controller ( a complicated hardware design) but I think I have a clean software implementation to manage it.
> 

It would be good to add that MPP do have more functionality than normal GPIO. So, I expect that in future
we might have few exported APIs beyond gpiolib integration. 

---Trilok Soni

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

WARNING: multiple messages have this Message-ID (diff)
From: tsoni@codeaurora.org (Trilok Soni)
To: linux-arm-kernel@lists.infradead.org
Subject: [Qualcomm PM8921 MFD v2 4/6] mfd: pm8xxx-mpp: Add pm8xxx MPP driver
Date: Thu, 10 Mar 2011 09:26:38 +0530	[thread overview]
Message-ID: <4D784BF6.1040700@codeaurora.org> (raw)
In-Reply-To: <4D784730.4010307@codeaurora.org>

Hi Abhi,

On 3/10/2011 9:06 AM, Abhijeet Dharmapurikar wrote:
> Mark Brown wrote:
>> On Mon, Mar 07, 2011 at 10:09:48PM -0800, adharmap at codeaurora.org wrote:
>>
>>> +    mpp_chip->gpio_chip.label = PM8XXX_MPP_DEV_NAME;
>>> +    mpp_chip->gpio_chip.direction_input = pm8xxx_mpp_dir_input;
>>> +    mpp_chip->gpio_chip.direction_output = pm8xxx_mpp_dir_output;
>>> +    mpp_chip->gpio_chip.to_irq = pm8xxx_mpp_to_irq;
>>> +    mpp_chip->gpio_chip.get = pm8xxx_mpp_get;
>>> +    mpp_chip->gpio_chip.set = pm8xxx_mpp_set;
>>> +    mpp_chip->gpio_chip.dbg_show = pm8xxx_mpp_dbg_show;
>>> +    mpp_chip->gpio_chip.ngpio = pdata->core_data.nmpps;
>>> +    mpp_chip->gpio_chip.can_sleep = 1;
>>> +    mpp_chip->gpio_chip.dev = &pdev->dev;
>>> +    mpp_chip->gpio_chip.base = pdata->mpp_base;
>>
>> It's seems really odd that you're adding gpiolib stuff here when you've
>> also got a separate gpiolib driver.  Possibly this all shouldn't
>> actually be split up as much as it is - there's also the issue with the
>> gpiolib driver needing to peer into the interrupt controller.  It might
>> simplify the code if things were merged more.
> 
> I dont think merging code will help here. gpio lines and mpp lines are very different piece of hardware,they have a different register map and different config attributes. They do fall under the generic 'gpiolib' umbrella, but it seems clean to keep them separate.
> 
> I agree with you that gpio code needs to call on interrupt controller ( a complicated hardware design) but I think I have a clean software implementation to manage it.
> 

It would be good to add that MPP do have more functionality than normal GPIO. So, I expect that in future
we might have few exported APIs beyond gpiolib integration. 

---Trilok Soni

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

  reply	other threads:[~2011-03-10  3:56 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-08  6:09 [Qualcomm PM8921 MFD v2 0/6] pmic 8921 core and subdevices adharmap
2011-03-08  6:09 ` adharmap at codeaurora.org
2011-03-08  6:09 ` [Qualcomm PM8921 MFD v2 1/6] mfd: pm8921: Add PMIC 8921 core driver adharmap
2011-03-08  6:09   ` adharmap at codeaurora.org
2011-03-08 17:58   ` Randy Dunlap
2011-03-08 17:58     ` Randy Dunlap
2011-03-08  6:09 ` [Qualcomm PM8921 MFD v2 2/6] mfd: pm8xxx: Add irq support adharmap
2011-03-08  6:09   ` adharmap at codeaurora.org
2011-03-08 12:04   ` Thomas Gleixner
2011-03-08 12:04     ` Thomas Gleixner
2011-03-09  5:21     ` Abhijeet Dharmapurikar
2011-03-09  5:21       ` Abhijeet Dharmapurikar
2011-03-10 10:32       ` Thomas Gleixner
2011-03-10 10:32         ` Thomas Gleixner
2011-03-11  4:43         ` Abhijeet Dharmapurikar
2011-03-11  4:43           ` Abhijeet Dharmapurikar
2011-03-11 17:57           ` Thomas Gleixner
2011-03-11 17:57             ` Thomas Gleixner
2011-03-11 19:02             ` Abhijeet Dharmapurikar
2011-03-11 19:02               ` Abhijeet Dharmapurikar
2011-03-11 19:43               ` Thomas Gleixner
2011-03-11 19:43                 ` Thomas Gleixner
2011-03-11 19:57                 ` Mark Brown
2011-03-11 19:57                   ` Mark Brown
2011-03-11 20:12                   ` Thomas Gleixner
2011-03-11 20:12                     ` Thomas Gleixner
2011-03-11 20:06                 ` Abhijeet Dharmapurikar
2011-03-11 20:06                   ` Abhijeet Dharmapurikar
2011-03-11 20:37                   ` Thomas Gleixner
2011-03-11 20:37                     ` Thomas Gleixner
2011-03-12  0:13                     ` Abhijeet Dharmapurikar
2011-03-12  0:13                       ` Abhijeet Dharmapurikar
2011-03-12 10:18                     ` [tip:irq/core] genirq: Add chip flag to force mask on suspend tip-bot for Thomas Gleixner
2011-03-08  6:09 ` [Qualcomm PM8921 MFD v2 3/6] gpio: pm8xxx-gpio: Add pm8xxx gpio driver adharmap
2011-03-08  6:09   ` adharmap at codeaurora.org
2011-03-12  9:36   ` Grant Likely
2011-03-12  9:36     ` Grant Likely
2011-03-16 18:55     ` Abhijeet Dharmapurikar
2011-03-16 18:55       ` Abhijeet Dharmapurikar
2011-03-16 19:54       ` Grant Likely
2011-03-16 19:54         ` Grant Likely
2011-03-16 19:56         ` Mark Brown
2011-03-16 19:56           ` Mark Brown
2011-03-16 23:06           ` Grant Likely
2011-03-16 23:06             ` Grant Likely
2011-03-08  6:09 ` [Qualcomm PM8921 MFD v2 4/6] mfd: pm8xxx-mpp: Add pm8xxx MPP driver adharmap
2011-03-08  6:09   ` adharmap at codeaurora.org
2011-03-08 23:30   ` Mark Brown
2011-03-08 23:30     ` Mark Brown
2011-03-10  3:36     ` Abhijeet Dharmapurikar
2011-03-10  3:36       ` Abhijeet Dharmapurikar
2011-03-10  3:56       ` Trilok Soni [this message]
2011-03-10  3:56         ` Trilok Soni
2011-03-10 15:13       ` Mark Brown
2011-03-10 15:13         ` Mark Brown
2011-03-08  6:09 ` [Qualcomm PM8921 MFD v2 5/6] MAINTAINERS: Add patterns for pmic 8921 files to MSM subsystem adharmap
2011-03-08  6:09   ` adharmap at codeaurora.org
2011-03-08 23:31   ` Mark Brown
2011-03-08 23:31     ` Mark Brown
2011-03-08  6:09 ` [Qualcomm PM8921 MFD v2 6/6] msm: board-8960: Add support for pm8921 adharmap
2011-03-08  6:09   ` adharmap at codeaurora.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D784BF6.1040700@codeaurora.org \
    --to=tsoni@codeaurora.org \
    --cc=adharmap@codeaurora.org \
    --cc=akpm@linux-foundation.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=bryanh@codeaurora.org \
    --cc=collinsd@codeaurora.org \
    --cc=davem@davemloft.net \
    --cc=davidb@codeaurora.org \
    --cc=dwalker@fifo99.com \
    --cc=grant.likely@secretlab.ca \
    --cc=gregkh@suse.de \
    --cc=joe@perches.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux.walleij@sterricsson.com \
    --cc=linux@arm.linux.org.uk \
    --cc=sameo@linux.intel.com \
    --cc=stepanm@codeaurora.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.