Re: [PATCH v3 0/5] mmc: Add access to RPMB partition
Hi Sundar, On 11/21/2012 12:02 PM, Sundar J. Dev wrote: > Hi Loic, > > On Wed, Nov 21, 2012 at 3:25 PM, Loic PALLARDY wrote: >> >>> >>> I am guessing that you would expect the userspace application too call >>> into the ioctl twice to take care of the 4& 5 >> Yes exactly, you have to first send your command and then to read the >> result, 2 IOCTLs are needed >> >>> and that might not be an >>> issue if there was no request processed for mmcqd i.e. no other >>> process/thread claims the host. But if that were to happen, then the >>> rpmb operation will fail - please let me know if this assumption or my >>> understanding of the spec is wrong. >> I tested this and I don't identify issue. >> I have Android running in parallel of my test and lot of other mmc >> accesses are performed in parallel, so my test suite doesn't guarantee >> that the 2 ioctls are contiguous. >> I had the same understanding of the RPMB specification, but after tests >> and discussion with our eMMC provider, it appears that RPMB state >> machine is independent of the rest. > I have seen the issue described by Krishna in one of the Micron eMMC > parts that I tested on. I captured the below CMD snapshot to explain > this better: > ---> * START RPMB TRANSACTION * > --> RPMB CMD6 - Switch to RPMB partition > --> RPMB CMD23 - Set BLK_CNT > --> RPMB CMD25 - Issue a Mult Blk Write > --> RPMB CMD13 - Issue Status CMD and and check for Write > completion > ---> * RPMB TRANSACTION ABORTED BY NON-RPMB ACCESS * > --> NON-RPMB CMD6 - Switch to Userdata partition > --> NON-RPMB CMD23 - Set BLK_CNT > --> NON-RPMB CMD25 - Issue Mutl Blk Write > --> NON-RPMB CMD12 - Issue Status CMD and and check for completion > ---> * SWITCH BACK TO RPMB PARTITION * > --> RPMB CMD6 - Switch back to RPMB partition > --> RPMB CMD23 - Set BLK_CNT > --> RPMB CMD18 - Issue a Mult Blk Read > The RPMB device returns GENERAL FAILURE in the Read data Packet. > Humm yes log is clear. Micron eMMC doesn't seem to accept interruption during RPMB sequence. Is it working when transfer is not interrupted? I tested on Toshiba and Sandisk without issue but it is not exhaustive enough. In your test, how many half sectors (256B) are you programming? I got some general failure when writing more than one half sector on some devices, but it has been explained by vendor. > Actually, the eMMC JEDEC spec is not very clear on what is the expected > behavior from eMMC chip if a rpmb sequence is interrupted by a non-rpmb > command sequence. It works fine on most Samsung and Toshiba > eMMC parts that I tested rpmb on. But this one Micron eMMC part showed > this problem. Yes unclear eMMC JEDEC spec about RPMB was reported by our eMMC provider. >>> >>> Similarly for rpmb write operation, these are the step involved >>> 1. Switch partition >>> 2. Set block count >>> 3. Write data frame - CMD25 to write the rpmb data frame with data >>> 4. Set block count >>> 5. Read the data - CMD25 to write rpmb data frame indicating that rpmb >>> result register is about to be read >>> 6. Set block count >>> 7. Read rpmb result - CMD18 to read the rpmb result register >>> >>> In the case of write, there are an additional two commands compared to >>> reads. Since all of these needs to be done in one shot, I believe the >>> current ioctl is not sufficient and this can be handled in the following >>> ways >> >> No needed to do it in one shot. You can send the write and check status >> later. >> I tested it, didn't detect any problem. >>> >>> 1. Extend the current ioctl to handle both cases >>> 2. Add a new ioctl cmd for rpmb requests >> It was my first implementation, but after internal STE review and eMMC >> check I merge it in existing ioctl. >> >> Please let me know if you detect any issue. >> >> Regards, >> Loic >>> >>> Personally I think adding another ioctl is a better way to do this since >>> the current ioctl will get cumbersome and technically the rpmb requests >>> are different kind of requests that need to be done atomically. I am >>> coding this up as a separate ioctl but before I post the patch, I wanted >>> feedback on this approach. >>> Solution must be driven by the most constrained device. In that case, I agree with Krishna, in that case a new ioctl seems to be the better approach. Regards, Loic >>> > > Thanks, > -- > Sundar Jayakumar Dev
Re: [PATCH] mmc: mmci: Support non-power-of-two block sizes for ux500v2 variant
On Wed, Nov 21, 2012 at 05:13:55PM +0100, Per Forlin wrote:
> On Wed, Nov 21, 2012 at 4:38 PM, Russell King - ARM Linux
> wrote:
> > On Fri, Oct 12, 2012 at 04:02:02PM +0200, Ulf Hansson wrote:
> >> /*
> >> + * Validate mmc prerequisites
> >> + */
> >> +static int mmci_validate_data(struct mmci_host *host,
> >> + struct mmc_data *data)
> >> +{
> >> + if (!data)
> >> + return 0;
> >> +
> >> + if (!host->variant->non_power_of_2_blksize &&
> >> + !is_power_of_2(data->blksz)) {
> >> + dev_err(mmc_dev(host->mmc),
> >> + "unsupported block size (%d bytes)\n", data->blksz);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (data->sg->offset & 3) {
> >> + dev_err(mmc_dev(host->mmc),
> >> + "unsupported alginment (0x%x)\n", data->sg->offset);
> >> + return -EINVAL;
> >> + }
> >
> > Why? What's the reasoning behind this suddenly introduced restriction?
> > readsl()/writesl() copes just fine with non-aligned pointers. It may be
> > that your DMA engine can not, but that's no business interfering with
> > non-DMA transfers, and no reason to fail such transfers.
> >
> > If your DMA engine can't do that then its your DMA engine code which
> > should refuse to prepare the transfer.
> >
> > Yes, that means problems with the way things are ordered - or it needs a
> > proper API where DMA engine can export these kinds of properties.
> The alignment constraint is related to PIO, sg_miter and that FIFO
> access must be done with 4 bytes.
Total claptrap. No it isn't.
- sg_miter just deals with bytes, and number of bytes transferred; there
is no word assumptions in that code. Indeed many ATA disks transfer
by half-word accesses so such a restriction would be insane.
- the FIFO access itself needs to be 32-bit words, so readsl or writesl
(or their io* equivalents must be used).
- but - and this is the killer item to your argument as I said above -
readsl and writesl _can_ take misaligned pointers and cope with them
fine.
The actual alignment of the buffer address is totally irrelevant here.
What isn't irrelevant is the _number_ of bytes to be transferred, but
that's something totally different and completely unrelated from
data->sg->offset.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: mmci: Support non-power-of-two block sizes for ux500v2 variant
On Wed, Nov 21, 2012 at 4:38 PM, Russell King - ARM Linux
wrote:
> On Fri, Oct 12, 2012 at 04:02:02PM +0200, Ulf Hansson wrote:
>> /*
>> + * Validate mmc prerequisites
>> + */
>> +static int mmci_validate_data(struct mmci_host *host,
>> + struct mmc_data *data)
>> +{
>> + if (!data)
>> + return 0;
>> +
>> + if (!host->variant->non_power_of_2_blksize &&
>> + !is_power_of_2(data->blksz)) {
>> + dev_err(mmc_dev(host->mmc),
>> + "unsupported block size (%d bytes)\n", data->blksz);
>> + return -EINVAL;
>> + }
>> +
>> + if (data->sg->offset & 3) {
>> + dev_err(mmc_dev(host->mmc),
>> + "unsupported alginment (0x%x)\n", data->sg->offset);
>> + return -EINVAL;
>> + }
>
> Why? What's the reasoning behind this suddenly introduced restriction?
> readsl()/writesl() copes just fine with non-aligned pointers. It may be
> that your DMA engine can not, but that's no business interfering with
> non-DMA transfers, and no reason to fail such transfers.
>
> If your DMA engine can't do that then its your DMA engine code which
> should refuse to prepare the transfer.
>
> Yes, that means problems with the way things are ordered - or it needs a
> proper API where DMA engine can export these kinds of properties.
The alignment constraint is related to PIO, sg_miter and that FIFO
access must be done with 4 bytes.
For a 8k buffer sg miter may return 3 buffer
1. 7 bytes
2. 4096
3. 4089
DMA can handle this because it will treat this a one buffer being 8 k.
PIO will do three transfer due to sg_miter (7, 4096, 4089).
One could change the driver to not use sg_miter and just access the 8k
buffer directly to avoid the issue.
BR
Per
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: mmci: Support non-power-of-two block sizes for ux500v2 variant
On Fri, Oct 12, 2012 at 04:02:02PM +0200, Ulf Hansson wrote:
> /*
> + * Validate mmc prerequisites
> + */
> +static int mmci_validate_data(struct mmci_host *host,
> + struct mmc_data *data)
> +{
> + if (!data)
> + return 0;
> +
> + if (!host->variant->non_power_of_2_blksize &&
> + !is_power_of_2(data->blksz)) {
> + dev_err(mmc_dev(host->mmc),
> + "unsupported block size (%d bytes)\n", data->blksz);
> + return -EINVAL;
> + }
> +
> + if (data->sg->offset & 3) {
> + dev_err(mmc_dev(host->mmc),
> + "unsupported alginment (0x%x)\n", data->sg->offset);
> + return -EINVAL;
> + }
Why? What's the reasoning behind this suddenly introduced restriction?
readsl()/writesl() copes just fine with non-aligned pointers. It may be
that your DMA engine can not, but that's no business interfering with
non-DMA transfers, and no reason to fail such transfers.
If your DMA engine can't do that then its your DMA engine code which
should refuse to prepare the transfer.
Yes, that means problems with the way things are ordered - or it needs a
proper API where DMA engine can export these kinds of properties.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: mmci: Fixup and cleanup code for DMA handling
On 16 October 2012 13:44, Ulf Hansson wrote: > On 16 October 2012 10:17, Linus Walleij wrote: >> On Tue, Oct 16, 2012 at 9:16 AM, Ulf Hansson wrote: >> >>> This code has not been tested on a "legacy" ARM PL180 but only for >>> ux500 boards. Even if it should affect DMA handling we should test >>> this properly. Would be great if you were able to help out, I guess >>> you still have available hardware for these tests? >> >> I can test the Integrator/CP and Realview in IRQ/PIO mode and >> the U300 in DMA mode. > > Great! > >> >> I need some help to provoke errorpath in DMA mode though, >> so any hints are welcome... > > One good option could be to stress test card insert/removal sequences, > while at the same time doing read/write requests. > >> >> Yours, >> Linus Walleij > > Kind regards > Ulf Hansson Just a kind reminder for Linus; did you manage to do some tests for these patches? Kind regards Ulf Hansson -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] rtsx patch for for-next branch in MFD tree
Hi Wei, On Tue, Nov 20, 2012 at 11:24:29AM +0800, wei_w...@realsil.com.cn wrote: > From: Wei WANG > > Wei WANG (2): > mmc/host/rtsx: Configure SD_CFG2 register in sd_rw_multi > mmc/host/rtsx: Explicitely include slab.h in rtsx_pci_sdmmc.c > > drivers/mmc/host/rtsx_pci_sdmmc.c |2 ++ > 1 file changed, 2 insertions(+) Applied, thanks. Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC: eMMC
Recently there have been a number of patches to sdhci.c and discussions on regulators for folks using eMMC. I would appreciate any feedback. The opinions below are my own. eMMC is a hardware device. It is NOT voltage configurable in any real sense other then turning on/off the voltage, The board designer is supposed to read the data sheet and hook things up. It is somewhat unclear to me how having a dummy regulator really helps. Given that -- voltage checking for vmmc or vmmcq is not meaningful. eMMC either works or does not. The testing for vccq/vcc has no meaning since it cannot be changed. In fact the samsung eMMC we used for DDR worked at 2.8v. Given this -- we have a chicken and egg situation in sdhci.c if we are in this code and the kernel was on eMMC obviously the system is working. If we booted the kernel from say SPI then hopefully the boot code has set up the voltage rails correctly. I would argue that for eMMC the regulator structure should not be exposed to sdhci.c and everything would just work. SD/UHS cards are a completely different matter and regulators make complete sense. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 077/493] mmc: remove use of __devexit_p
On Mon, 19 Nov 2012, Bill Pemberton wrote: drivers/mmc/host/sh_mobile_sdhi.c | 2 +- drivers/mmc/host/tmio_mmc.c| 2 +- Acked-by: Guennadi Liakhovetski Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL v2] at91: fixes for 3.7-rc7
On 11/21/2012 08:03 AM, Olof Johansson : > Hi, > > > On Tue, Nov 20, 2012 at 09:59:27AM +0100, Nicolas Ferre wrote: >> Arnd, Olof, >> >> Just for the record, I do not want to put pressure at a such late time in >> the 3.7-rc process. So, I just reworked that pull-request because the >> previous >> one was wrong: >> - wrong patch content (DT nodes with wrong size) >> - not all tags in patches (Jean-Christophe and Arnd tags were missing...) >> >> Just to start from a sane base if I have to rebase this work for 3.8, I let >> you know >> that I have updated this tag... >> >> The following changes since commit 641f3ce64b050961d454a0716bb6dbf528315aac: >> >> ARM: at91/usbh: fix overcurrent gpio setup (2012-11-16 10:46:29 +0100) >> >> are available in the git repository at: >> >> git://github.com/at91linux/linux-at91.git tags/at91-fixes > > The new patches seem to belong in an at91/dt branch, not in a fixes one. > > I can pull in the previous fixes branch as an at91/fixes-non-critical for 3.8 > if you want. There's no need to rebase them for this, is there? What is the > pinctrl dependency that you are talking about, are some of these patches > needed > as prerequisites for pinctrl changes or the other way around? > > Sorry if I've missed more elaborate emails on this and are asking repeat > questions. ;) No worries Olof, I might have been more precise in the subject of my email: I have made up my mind and consider this material for 3.8. As for the relation with pinctrl, we have made big modification to the layout of some dtsi/dts there and it would make everyones' life easier if we queue these dt/mmc changes on top of the current pinctrl tree... Moreover, Jean-Christophe plans to add the pinctrl part of these additions on top of the modification present in this pull-request: one more reason to queue them in pinctrl git tree. So, in brief: forget this pull-request (and the one that it replaces obviously). Thanks, bye, -- Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/5] mmc: Add access to RPMB partition
Hi Loic, On Wed, Nov 21, 2012 at 3:25 PM, Loic PALLARDY wrote: > > Hi Krishna, > > On 11/20/2012 07:54 PM, Krishna Konda wrote: > > > > Loic/Linus/Chris, I think the IOCTL is not complete in terms of handling > > the RPMB requests. Here is why I think that is - let me know your > > opinion > > > > There are four request types that are needed to be supported - two under > > read category and two under write. They are > > > > Reads > > --- > > 1. Read Write Counter > > 2. Authenticated data read > > > > > > Writes > > --- > > 1. Provision RPMB key (though it might be done in a secure environment) > > 2. Authenticated data read > Agree > > > > While its given that the rpmb data frames are going to have that > > information encoded in it and the frames will be generated by a secure > > piece of code, the request types can be classified as above. > > > > The ioctl interface to do this but currently that does the following > > 1. Switch partition > > 2. Set block count > > 3. One command - whatever is passed in by the userspace application. > > > > So here are the set of commands that need to happen in a rpmb read > > operation > > 1. Switch partition > > 2. Set block count > > 3. Write data frame - CMD25 to write the rpmb data frame > > 4. Set block count > > 5. Read the data - CMD18 to do the actual read > > > > I am guessing that you would expect the userspace application too call > > into the ioctl twice to take care of the 4& 5 > Yes exactly, you have to first send your command and then to read the > result, 2 IOCTLs are needed > > > and that might not be an > > issue if there was no request processed for mmcqd i.e. no other > > process/thread claims the host. But if that were to happen, then the > > rpmb operation will fail - please let me know if this assumption or my > > understanding of the spec is wrong. > I tested this and I don't identify issue. > I have Android running in parallel of my test and lot of other mmc > accesses are performed in parallel, so my test suite doesn't guarantee > that the 2 ioctls are contiguous. > I had the same understanding of the RPMB specification, but after tests > and discussion with our eMMC provider, it appears that RPMB state > machine is independent of the rest. I have seen the issue described by Krishna in one of the Micron eMMC parts that I tested on. I captured the below CMD snapshot to explain this better: ---> * START RPMB TRANSACTION * --> RPMB CMD6 - Switch to RPMB partition --> RPMB CMD23 - Set BLK_CNT --> RPMB CMD25 - Issue a Mult Blk Write --> RPMB CMD13 - Issue Status CMD and and check for Write completion ---> * RPMB TRANSACTION ABORTED BY NON-RPMB ACCESS * --> NON-RPMB CMD6 - Switch to Userdata partition --> NON-RPMB CMD23 - Set BLK_CNT --> NON-RPMB CMD25 - Issue Mutl Blk Write --> NON-RPMB CMD12 - Issue Status CMD and and check for completion ---> * SWITCH BACK TO RPMB PARTITION * --> RPMB CMD6 - Switch back to RPMB partition --> RPMB CMD23 - Set BLK_CNT --> RPMB CMD18 - Issue a Mult Blk Read The RPMB device returns GENERAL FAILURE in the Read data Packet. Actually, the eMMC JEDEC spec is not very clear on what is the expected behavior from eMMC chip if a rpmb sequence is interrupted by a non-rpmb command sequence. It works fine on most Samsung and Toshiba eMMC parts that I tested rpmb on. But this one Micron eMMC part showed this problem. > > > > Similarly for rpmb write operation, these are the step involved > > 1. Switch partition > > 2. Set block count > > 3. Write data frame - CMD25 to write the rpmb data frame with data > > 4. Set block count > > 5. Read the data - CMD25 to write rpmb data frame indicating that rpmb > > result register is about to be read > > 6. Set block count > > 7. Read rpmb result - CMD18 to read the rpmb result register > > > > In the case of write, there are an additional two commands compared to > > reads. Since all of these needs to be done in one shot, I believe the > > current ioctl is not sufficient and this can be handled in the following > > ways > > No needed to do it in one shot. You can send the write and check status > later. > I tested it, didn't detect any problem. > > > > 1. Extend the current ioctl to handle both cases > > 2. Add a new ioctl cmd for rpmb requests > It was my first implementation, but after internal STE review and eMMC > check I merge it in existing ioctl. > > Please let me know if you detect any issue. > > Regards, > Loic > > > > Personally I think adding another ioctl is a better way to do this since > > the current ioctl will get cumbersome and technically the rpmb requests > > are different kind of requests that need to be done atomically. I am > > coding this up as a separate ioctl but before I post the patch, I wanted > > feedback on this approach. > > > > Thanks, -- Sundar Jayakumar Dev -- To unsubscribe from this list: send the li
Re: [PATCH v2 2/2] mmc: host: sdhci-s3c: Add support for pinctrl
On 16 November 2012 19:58, Tomasz Figa wrote:
> This patch adds support for pin configuration using pinctrl subsystem
> to the sdhci-s3c driver.
>
> Signed-off-by: Tomasz Figa
> ---
> .../devicetree/bindings/mmc/samsung-sdhci.txt| 20
> +---
> drivers/mmc/host/sdhci-s3c.c | 12 ++--
> 2 files changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
> b/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
> index 630a7d7..97e9e31 100644
> --- a/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/samsung-sdhci.txt
> @@ -12,10 +12,6 @@ is used. The Samsung's SDHCI controller bindings extends
> this as listed below.
> [A] The property "samsung,cd-pinmux-gpio" can be used as stated in the
> "Optional Board Specific Properties" section below.
>
> -[B] If core card-detect bindings and "samsung,cd-pinmux-gpio" property
> -is not specified, it is assumed that there is no card detection
> -mechanism used.
> -
> Required SoC Specific Properties:
> - compatible: should be one of the following
>- "samsung,s3c6410-sdhci": For controllers compatible with s3c6410 sdhci
> @@ -24,14 +20,18 @@ Required SoC Specific Properties:
> controller.
>
> Required Board Specific Properties:
> -- gpios: Should specify the gpios used for clock, command and data lines. The
> - gpio specifier format depends on the gpio controller.
> +- Samsung GPIO variant (will be completely replaced by pinctrl):
> + - gpios: Should specify the gpios used for clock, command and data lines.
> The
> +gpio specifier format depends on the gpio controller.
> +- Pinctrl variant (preferred if available):
> + - pinctrl-0: Should specify pin control groups used for this controller.
> + - pinctrl-names: Should contain only one value - "default".
>
> Optional Board Specific Properties:
> - samsung,cd-pinmux-gpio: Specifies the card detect line that is routed
>through a pinmux to the card-detect pin of the card slot. This property
>should be used only if none of the mmc core card-detect properties are
> - used.
> + used. Only for Samsung GPIO variant.
>
> Example:
> sdhci@1253 {
> @@ -40,12 +40,18 @@ Example:
> interrupts = <0 75 0>;
> bus-width = <4>;
> cd-gpios = <&gpk2 2 2 3 3>;
> +
> + /* Samsung GPIO variant */
> gpios = <&gpk2 0 2 0 3>, /* clock line */
> <&gpk2 1 2 0 3>, /* command line */
> <&gpk2 3 2 3 3>, /* data line 0 */
> <&gpk2 4 2 3 3>, /* data line 1 */
> <&gpk2 5 2 3 3>, /* data line 2 */
> <&gpk2 6 2 3 3>; /* data line 3 */
> +
> + /* Pinctrl variant */
> + pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4>;
> + pinctrl-names = "default";
> };
nit: there could have been one example each for gpio and pinctrl
variant instead of putting both into one example node.
>
> Note: This example shows both SoC specific and board specific
> properties
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 75f85fd..6161162 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -57,6 +58,7 @@ struct sdhci_s3c {
> int ext_cd_irq;
> int ext_cd_gpio;
> int *gpios;
> + struct pinctrl *pctrl;
>
> struct clk *clk_io;
> struct clk *clk_bus[MAX_BUS_CLK];
> @@ -477,8 +479,9 @@ static int __devinit sdhci_s3c_parse_dt(struct device
> *dev,
> return -EINVAL;
> }
>
> - dev_info(dev, "assuming no card detect line available\n");
> - pdata->cd_type = S3C_SDHCI_CD_NONE;
> + /* assuming internal card detect that will be configured by pinctrl */
> + pdata->cd_type = S3C_SDHCI_CD_INTERNAL;
> + goto setup_bus;
>
> found_cd:
> if (pdata->cd_type == S3C_SDHCI_CD_GPIO) {
> @@ -496,6 +499,9 @@ static int __devinit sdhci_s3c_parse_dt(struct device
> *dev,
> }
>
> setup_bus:
> + if (!IS_ERR(ourhost->pctrl))
> + return 0;
> +
> /* get the gpios for command, clock and data lines */
> for (cnt = 0; cnt < NUM_GPIOS(pdata->max_width); cnt++) {
> gpio = of_get_gpio(node, cnt);
> @@ -574,6 +580,8 @@ static int __devinit sdhci_s3c_probe(struct
> platform_device *pdev)
> goto err_pdata_io_clk;
> }
>
> + sc->pctrl = devm_pinctrl_get_select_default(&pdev->dev);
> +
> if (pdev->dev.of_node) {
> ret = sdhci_s3c_parse_dt(&pdev->dev, host, pdata);
>
Re: [PATCH v2 1/2] mmc: host: sdhci-s3c: Use devm_gpio_request to request GPIOs
On 16 November 2012 19:58, Tomasz Figa wrote:
> The set of GPIO pins used by sdhci-s3c driver varies between
> configurations, such as card detect method, pinctrl availability, etc.
> This overly complicates the code requesting and freeing GPIO pins, which
> must check which pins are used, when freeing them.
>
> This patch modifies the sdhci-s3c driver to use devm_gpio_request to
> free requested pins automatically after unbinding the driver.
>
> Signed-off-by: Tomasz Figa
> ---
> drivers/mmc/host/sdhci-s3c.c | 40 +---
> 1 file changed, 9 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 2903949..75f85fd 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -406,7 +406,7 @@ static void sdhci_s3c_setup_card_detect_gpio(struct
> sdhci_s3c *sc)
> struct s3c_sdhci_platdata *pdata = sc->pdata;
> struct device *dev = &sc->pdev->dev;
>
> - if (gpio_request(pdata->ext_cd_gpio, "SDHCI EXT CD") == 0) {
> + if (devm_gpio_request(dev, pdata->ext_cd_gpio, "SDHCI EXT CD") == 0) {
> sc->ext_cd_gpio = pdata->ext_cd_gpio;
> sc->ext_cd_irq = gpio_to_irq(pdata->ext_cd_gpio);
> if (sc->ext_cd_irq &&
> @@ -487,7 +487,7 @@ static int __devinit sdhci_s3c_parse_dt(struct device
> *dev,
> if (of_get_property(node, "cd-inverted", NULL))
> pdata->ext_cd_gpio_invert = 1;
> } else if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL) {
> - ret = gpio_request(gpio, "sdhci-cd");
> + ret = devm_gpio_request(dev, gpio, "sdhci-cd");
> if (ret) {
> dev_err(dev, "card detect gpio request failed\n");
> return -EINVAL;
> @@ -501,28 +501,20 @@ static int __devinit sdhci_s3c_parse_dt(struct device
> *dev,
> gpio = of_get_gpio(node, cnt);
> if (!gpio_is_valid(gpio)) {
> dev_err(dev, "invalid gpio[%d]\n", cnt);
> - goto err_free_dt_cd_gpio;
> + return -EINVAL;
> }
> ourhost->gpios[cnt] = gpio;
> }
>
> for (cnt = 0; cnt < NUM_GPIOS(pdata->max_width); cnt++) {
> - ret = gpio_request(ourhost->gpios[cnt], "sdhci-gpio");
> + ret = devm_gpio_request(dev, ourhost->gpios[cnt],
> "sdhci-gpio");
> if (ret) {
> dev_err(dev, "gpio[%d] request failed\n", cnt);
> - goto err_free_dt_gpios;
> + return -EINVAL;
> }
> }
>
> return 0;
> -
> - err_free_dt_gpios:
> - while (--cnt >= 0)
> - gpio_free(ourhost->gpios[cnt]);
> - err_free_dt_cd_gpio:
> - if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL)
> - gpio_free(ourhost->ext_cd_gpio);
> - return -EINVAL;
> }
> #else
> static int __devinit sdhci_s3c_parse_dt(struct device *dev,
> @@ -579,13 +571,13 @@ static int __devinit sdhci_s3c_probe(struct
> platform_device *pdev)
> pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> if (!pdata) {
> ret = -ENOMEM;
> - goto err_pdata;
> + goto err_pdata_io_clk;
> }
>
> if (pdev->dev.of_node) {
> ret = sdhci_s3c_parse_dt(&pdev->dev, host, pdata);
> if (ret)
> - goto err_pdata;
> + goto err_pdata_io_clk;
> } else {
> memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
> sc->ext_cd_gpio = -1; /* invalid gpio number */
> @@ -603,7 +595,7 @@ static int __devinit sdhci_s3c_probe(struct
> platform_device *pdev)
> if (IS_ERR(sc->clk_io)) {
> dev_err(dev, "failed to get io clock\n");
> ret = PTR_ERR(sc->clk_io);
> - goto err_io_clk;
> + goto err_pdata_io_clk;
> }
>
> /* enable the local io clock and keep it running for the moment. */
> @@ -765,13 +757,7 @@ static int __devinit sdhci_s3c_probe(struct
> platform_device *pdev)
> clk_disable(sc->clk_io);
> clk_put(sc->clk_io);
>
> - err_io_clk:
> - for (ptr = 0; ptr < NUM_GPIOS(sc->pdata->max_width); ptr++)
> - gpio_free(sc->gpios[ptr]);
> - if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL)
> - gpio_free(sc->ext_cd_gpio);
> -
> - err_pdata:
> + err_pdata_io_clk:
> sdhci_free_host(host);
>
> return ret;
> @@ -790,9 +776,6 @@ static int __devexit sdhci_s3c_remove(struct
> platform_device *pdev)
> if (sc->ext_cd_irq)
> free_irq(sc->ext_cd_irq, sc);
>
> - if (gpio_is_valid(sc->ext_cd_gpio))
> - gpio_free(sc->ext_cd_gpio);
> -
> #ifdef CONFIG_PM_RUNTIME
> clk_enable(sc-
[PATCH 1/1] mmc:atmel-mci: use devm_gpio_request/free and configure the pin corrently
as on DT we do not configure the pin via AT91 custom pinmux as before
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD
Cc: linux-mmc@vger.kernel.org
Cc: Ludovic Desroches
---
based on next-20121115
Best Regards,
J.
drivers/mmc/host/atmel-mci.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 767706b..d97771d 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -2108,6 +2108,7 @@ static int __init atmci_init_slot(struct atmel_mci *host,
{
struct mmc_host *mmc;
struct atmel_mci_slot *slot;
+ int ret;
mmc = mmc_alloc_host(sizeof(struct atmel_mci_slot), &host->pdev->dev);
if (!mmc)
@@ -2161,12 +2162,20 @@ static int __init atmci_init_slot(struct atmel_mci
*host,
/* Assume card is present initially */
set_bit(ATMCI_CARD_PRESENT, &slot->flags);
if (gpio_is_valid(slot->detect_pin)) {
- if (gpio_request(slot->detect_pin, "mmc_detect")) {
- dev_dbg(&mmc->class_dev, "no detect pin available\n");
- slot->detect_pin = -EBUSY;
- } else if (gpio_get_value(slot->detect_pin) ^
- slot->detect_is_active_high) {
- clear_bit(ATMCI_CARD_PRESENT, &slot->flags);
+ ret = devm_gpio_request(&mmc->class_dev, slot->detect_pin,
"mmc_detect");
+ if (ret) {
+ dev_warn(&mmc->class_dev, "can't request detect pin\n");
+ slot->detect_pin = ret;
+ } else {
+ ret = gpio_direction_input(slot->detect_pin);
+ if (ret) {
+ dev_err(&mmc->class_dev, "can't set detect pin
direction\n");
+ devm_gpio_free(&mmc->class_dev,
slot->detect_pin);
+ slot->detect_pin = -ret;
+ } else if (gpio_get_value(slot->detect_pin) ^
+ slot->detect_is_active_high) {
+ clear_bit(ATMCI_CARD_PRESENT, &slot->flags);
+ }
}
}
@@ -2174,9 +2183,17 @@ static int __init atmci_init_slot(struct atmel_mci *host,
mmc->caps |= MMC_CAP_NEEDS_POLL;
if (gpio_is_valid(slot->wp_pin)) {
- if (gpio_request(slot->wp_pin, "mmc_wp")) {
- dev_dbg(&mmc->class_dev, "no WP pin available\n");
- slot->wp_pin = -EBUSY;
+ ret = devm_gpio_request(&mmc->class_dev, slot->wp_pin,
"mmc_wp");
+ if (ret) {
+ dev_warn(&mmc->class_dev, "no WP pin available\n");
+ slot->wp_pin = ret;
+ } else {
+ ret = gpio_direction_output(slot->wp_pin, 0);
+ if (ret) {
+ dev_err(&mmc->class_dev, "can't set WP pin
direction\n");
+ devm_gpio_free(&mmc->class_dev, slot->wp_pin);
+ slot->wp_pin = ret;
+ }
}
}
@@ -2197,8 +2214,8 @@ static int __init atmci_init_slot(struct atmel_mci *host,
dev_dbg(&mmc->class_dev,
"could not request IRQ %d for detect pin\n",
gpio_to_irq(slot->detect_pin));
- gpio_free(slot->detect_pin);
- slot->detect_pin = -EBUSY;
+ devm_gpio_free(&mmc->class_dev, slot->detect_pin);
+ slot->detect_pin = ret;
}
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/2] mmc: host: sdhci-s3c: Add support for pinctrl interface
Hi, On Friday 16 of November 2012 15:28:15 Tomasz Figa wrote: > This series intends to add support for pin configuration using pin > control interface. > > First patch cleans up GPIO requesting and freeing in the driver to > simplify adding pin control support. > > Second patch adds pin control support to the driver. > > Changes since v1: > - fixed build error because of incorrect merge > > Tomasz Figa (2): > mmc: host: sdhci-s3c: Use devm_gpio_request to request GPIOs > mmc: host: sdhci-s3c: Add support for pinctrl > > .../devicetree/bindings/mmc/samsung-sdhci.txt | 20 ++--- > drivers/mmc/host/sdhci-s3c.c | 52 > -- 2 files changed, 32 insertions(+), 40 > deletions(-) I know it's less than a week since I posted this series, but time is running out, while it would be nice to get this included in 3.8, because it's required for all Exynos4 boards using MMC. Best regards, -- Tomasz Figa Samsung Poland R&D Center SW Solution Development, Linux Platform -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator: add missing prototype for regulator_is_supported_voltage
On Wed, Nov 21, 2012 at 06:06:12PM +0800, Eric Miao wrote: > Yep, that's fine. Applied, thanks. signature.asc Description: Digital signature
Re: [PATCH] regulator: add missing prototype for regulator_is_supported_voltage
On Wed, Nov 21, 2012 at 6:05 PM, Mark Brown wrote: > On Wed, Nov 21, 2012 at 05:45:57PM +0800, Eric Miao wrote: >> git-send-email stopped working so I have to use the web interface. >> Could you help >> try the attached one? > > OK. I take it's OK to add your signoff as well (you should always add > this for patches you forward)? Yep, that's fine. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator: add missing prototype for regulator_is_supported_voltage
On Wed, Nov 21, 2012 at 05:45:57PM +0800, Eric Miao wrote: > git-send-email stopped working so I have to use the web interface. > Could you help > try the attached one? OK. I take it's OK to add your signoff as well (you should always add this for patches you forward)? signature.asc Description: Digital signature
Re: [PATCH v3 0/5] mmc: Add access to RPMB partition
Hi Krishna, On 11/20/2012 07:54 PM, Krishna Konda wrote: > > Loic/Linus/Chris, I think the IOCTL is not complete in terms of handling > the RPMB requests. Here is why I think that is - let me know your > opinion > > There are four request types that are needed to be supported - two under > read category and two under write. They are > > Reads > --- > 1. Read Write Counter > 2. Authenticated data read > > > Writes > --- > 1. Provision RPMB key (though it might be done in a secure environment) > 2. Authenticated data read Agree > > While its given that the rpmb data frames are going to have that > information encoded in it and the frames will be generated by a secure > piece of code, the request types can be classified as above. > > The ioctl interface to do this but currently that does the following > 1. Switch partition > 2. Set block count > 3. One command - whatever is passed in by the userspace application. > > So here are the set of commands that need to happen in a rpmb read > operation > 1. Switch partition > 2. Set block count > 3. Write data frame - CMD25 to write the rpmb data frame > 4. Set block count > 5. Read the data - CMD18 to do the actual read > > I am guessing that you would expect the userspace application too call > into the ioctl twice to take care of the 4& 5 Yes exactly, you have to first send your command and then to read the result, 2 IOCTLs are needed > and that might not be an > issue if there was no request processed for mmcqd i.e. no other > process/thread claims the host. But if that were to happen, then the > rpmb operation will fail - please let me know if this assumption or my > understanding of the spec is wrong. I tested this and I don't identify issue. I have Android running in parallel of my test and lot of other mmc accesses are performed in parallel, so my test suite doesn't guarantee that the 2 ioctls are contiguous. I had the same understanding of the RPMB specification, but after tests and discussion with our eMMC provider, it appears that RPMB state machine is independent of the rest. > > Similarly for rpmb write operation, these are the step involved > 1. Switch partition > 2. Set block count > 3. Write data frame - CMD25 to write the rpmb data frame with data > 4. Set block count > 5. Read the data - CMD25 to write rpmb data frame indicating that rpmb > result register is about to be read > 6. Set block count > 7. Read rpmb result - CMD18 to read the rpmb result register > > In the case of write, there are an additional two commands compared to > reads. Since all of these needs to be done in one shot, I believe the > current ioctl is not sufficient and this can be handled in the following > ways No needed to do it in one shot. You can send the write and check status later. I tested it, didn't detect any problem. > > 1. Extend the current ioctl to handle both cases > 2. Add a new ioctl cmd for rpmb requests It was my first implementation, but after internal STE review and eMMC check I merge it in existing ioctl. Please let me know if you detect any issue. Regards, Loic > > Personally I think adding another ioctl is a better way to do this since > the current ioctl will get cumbersome and technically the rpmb requests > are different kind of requests that need to be done atomically. I am > coding this up as a separate ioctl but before I post the patch, I wanted > feedback on this approach. > >
Re: [PATCH] regulator: add missing prototype for regulator_is_supported_voltage
git-send-email stopped working so I have to use the web interface.
Could you help
try the attached one?
On Wed, Nov 21, 2012 at 5:43 PM, Mark Brown
wrote:
> On Wed, Nov 21, 2012 at 05:22:02PM +0800, Eric Miao wrote:
>
>> +static inline int regulator_count_voltages(struct regulator *regulator)
>> +{
>> + return 0;
>> +}
>
> Looks like your mailer has mangled everything so this won't apply.
0001-regulator-add-missing-prototype-for-regulator_is_sup.patch
Description: Binary data
Re: [PATCH] regulator: add missing prototype for regulator_is_supported_voltage
On Wed, Nov 21, 2012 at 05:22:02PM +0800, Eric Miao wrote:
> +static inline int regulator_count_voltages(struct regulator *regulator)
> +{
> + return 0;
> +}
Looks like your mailer has mangled everything so this won't apply.
signature.asc
Description: Digital signature
