From: Arnd Bergmann <arnd@arndb.de> To: Gerd Hoffmann <kraxel@redhat.com> Cc: linux-rpi-kernel@lists.infradead.org, Eric Anholt <eric@anholt.net>, Ulf Hansson <ulf.hansson@linaro.org>, Stephen Warren <swarren@wwwdotorg.org>, Lee Jones <lee@kernel.org>, open list <linux-kernel@vger.kernel.org>, "open list:MULTIMEDIA CARD (MMC), SECURE DIGITAL (SD) AND..." <linux-mmc@vger.kernel.org>, "moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH 21/32] mmc: bcm2835-sdhost: Add new driver for the internal SD controller. Date: Thu, 02 Jun 2016 00:18:52 +0200 [thread overview] Message-ID: <4715245.LFBOAeJdHc@wuerfel> (raw) In-Reply-To: <1464817421-8519-22-git-send-email-kraxel@redhat.com> On Wednesday, June 1, 2016 11:43:30 PM CEST Gerd Hoffmann wrote: > +static void bcm2835_sdhost_reset_internal(struct bcm2835_host *host) > +{ > + u32 temp; > + > + bcm2835_sdhost_set_power(host, false); > + > + bcm2835_sdhost_write(host, 0, SDCMD); > + bcm2835_sdhost_write(host, 0, SDARG); > + bcm2835_sdhost_write(host, 0xf00000, SDTOUT); > + bcm2835_sdhost_write(host, 0, SDCDIV); > + bcm2835_sdhost_write(host, 0x7f8, SDHSTS); /* Write 1s to clear */ > + bcm2835_sdhost_write(host, 0, SDHCFG); > + bcm2835_sdhost_write(host, 0, SDHBCT); > + bcm2835_sdhost_write(host, 0, SDHBLC); > + > + /* Limit fifo usage due to silicon bug */ > + temp = bcm2835_sdhost_read(host, SDEDM); > + temp &= ~((SDEDM_THRESHOLD_MASK << SDEDM_READ_THRESHOLD_SHIFT) | > + (SDEDM_THRESHOLD_MASK << SDEDM_WRITE_THRESHOLD_SHIFT)); > + temp |= (FIFO_READ_THRESHOLD << SDEDM_READ_THRESHOLD_SHIFT) | > + (FIFO_WRITE_THRESHOLD << SDEDM_WRITE_THRESHOLD_SHIFT); > + bcm2835_sdhost_write(host, temp, SDEDM); > + mdelay(10); > + bcm2835_sdhost_set_power(host, true); > + mdelay(10); Are you sure you cannot use msleep() here? > + host->clock = 0; > + bcm2835_sdhost_write(host, host->hcfg, SDHCFG); > + bcm2835_sdhost_write(host, host->cdiv, SDCDIV); > + mmiowb(); > +} What is the barrier for? Same question for all the other instances > + > +static void bcm2835_sdhost_reset(struct mmc_host *mmc) > +{ > + struct bcm2835_host *host = mmc_priv(mmc); > + unsigned long flags; > + > + spin_lock_irqsave(&host->lock, flags); > + bcm2835_sdhost_reset_internal(host); > + spin_unlock_irqrestore(&host->lock, flags); > +} The spinlock seems to only protect the reset from happening concurrently with bcm2835_sdhost_dma_complete() here. So if you ensure that this function is no longer pending, you can probably use msleep() above. > +static void bcm2835_sdhost_set_ios(struct mmc_host *mmc, struct mmc_ios *ios); Maybe rearrange the order of the functions to avoid forward declarations? > + spin_lock_init(&host->lock); > + > + if (IS_ERR_OR_NULL(host->dma_chan_tx) || > + IS_ERR_OR_NULL(host->dma_chan_rx)) { > + pr_err("%s: unable to initialise DMA channels. Falling back to PIO\n", > + mmc_hostname(mmc)); When are these ever NULL? > + /* Parse OF address directly to get the physical address for > + * DMA to our registers. > + */ > + host->phys_addr = be32_to_cpup(of_get_address(pdev->dev.of_node, 0, > + NULL, NULL)); This looks broken on 64-bit systems when #address-cells=<2>, or if there is an intermediate bus with a non-empty ranges property. What's wrong with platform_get_resource(pdev, IORESOURCE_MEM, 0)? Arnd
WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 21/32] mmc: bcm2835-sdhost: Add new driver for the internal SD controller. Date: Thu, 02 Jun 2016 00:18:52 +0200 [thread overview] Message-ID: <4715245.LFBOAeJdHc@wuerfel> (raw) In-Reply-To: <1464817421-8519-22-git-send-email-kraxel@redhat.com> On Wednesday, June 1, 2016 11:43:30 PM CEST Gerd Hoffmann wrote: > +static void bcm2835_sdhost_reset_internal(struct bcm2835_host *host) > +{ > + u32 temp; > + > + bcm2835_sdhost_set_power(host, false); > + > + bcm2835_sdhost_write(host, 0, SDCMD); > + bcm2835_sdhost_write(host, 0, SDARG); > + bcm2835_sdhost_write(host, 0xf00000, SDTOUT); > + bcm2835_sdhost_write(host, 0, SDCDIV); > + bcm2835_sdhost_write(host, 0x7f8, SDHSTS); /* Write 1s to clear */ > + bcm2835_sdhost_write(host, 0, SDHCFG); > + bcm2835_sdhost_write(host, 0, SDHBCT); > + bcm2835_sdhost_write(host, 0, SDHBLC); > + > + /* Limit fifo usage due to silicon bug */ > + temp = bcm2835_sdhost_read(host, SDEDM); > + temp &= ~((SDEDM_THRESHOLD_MASK << SDEDM_READ_THRESHOLD_SHIFT) | > + (SDEDM_THRESHOLD_MASK << SDEDM_WRITE_THRESHOLD_SHIFT)); > + temp |= (FIFO_READ_THRESHOLD << SDEDM_READ_THRESHOLD_SHIFT) | > + (FIFO_WRITE_THRESHOLD << SDEDM_WRITE_THRESHOLD_SHIFT); > + bcm2835_sdhost_write(host, temp, SDEDM); > + mdelay(10); > + bcm2835_sdhost_set_power(host, true); > + mdelay(10); Are you sure you cannot use msleep() here? > + host->clock = 0; > + bcm2835_sdhost_write(host, host->hcfg, SDHCFG); > + bcm2835_sdhost_write(host, host->cdiv, SDCDIV); > + mmiowb(); > +} What is the barrier for? Same question for all the other instances > + > +static void bcm2835_sdhost_reset(struct mmc_host *mmc) > +{ > + struct bcm2835_host *host = mmc_priv(mmc); > + unsigned long flags; > + > + spin_lock_irqsave(&host->lock, flags); > + bcm2835_sdhost_reset_internal(host); > + spin_unlock_irqrestore(&host->lock, flags); > +} The spinlock seems to only protect the reset from happening concurrently with bcm2835_sdhost_dma_complete() here. So if you ensure that this function is no longer pending, you can probably use msleep() above. > +static void bcm2835_sdhost_set_ios(struct mmc_host *mmc, struct mmc_ios *ios); Maybe rearrange the order of the functions to avoid forward declarations? > + spin_lock_init(&host->lock); > + > + if (IS_ERR_OR_NULL(host->dma_chan_tx) || > + IS_ERR_OR_NULL(host->dma_chan_rx)) { > + pr_err("%s: unable to initialise DMA channels. Falling back to PIO\n", > + mmc_hostname(mmc)); When are these ever NULL? > + /* Parse OF address directly to get the physical address for > + * DMA to our registers. > + */ > + host->phys_addr = be32_to_cpup(of_get_address(pdev->dev.of_node, 0, > + NULL, NULL)); This looks broken on 64-bit systems when #address-cells=<2>, or if there is an intermediate bus with a non-empty ranges property. What's wrong with platform_get_resource(pdev, IORESOURCE_MEM, 0)? Arnd
next prev parent reply other threads:[~2016-06-01 22:18 UTC|newest] Thread overview: 193+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <1464817421-8519-1-git-send-email-kraxel@redhat.com> 2016-06-01 21:43 ` [PATCH 01/32] arm64: Add platform selection for BCM2835 Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:49 ` Florian Fainelli 2016-06-01 21:49 ` Florian Fainelli 2016-06-02 6:45 ` Gerd Hoffmann 2016-06-02 6:45 ` Gerd Hoffmann 2016-06-02 16:25 ` Ray Jui 2016-06-02 16:25 ` Ray Jui 2016-06-02 16:48 ` Scott Branden 2016-06-02 16:48 ` Scott Branden 2016-06-02 17:12 ` Gerd Hoffmann 2016-06-02 17:12 ` Gerd Hoffmann 2016-06-02 17:21 ` Scott Branden 2016-06-02 17:21 ` Scott Branden 2016-06-02 17:36 ` Florian Fainelli 2016-06-02 17:36 ` Florian Fainelli 2016-06-02 17:30 ` Catalin Marinas 2016-06-02 17:30 ` Catalin Marinas 2016-06-01 21:43 ` [PATCH 02/32] arm64: Add BCM2835 support to the defconfig Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 03/32] irqchip: bcm2835: Avoid arch/arm-specific handle_IRQ Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-02 0:50 ` Jason Cooper 2016-06-02 0:50 ` Jason Cooper 2016-06-02 18:06 ` Eric Anholt 2016-06-02 18:06 ` Eric Anholt 2016-06-01 21:43 ` [PATCH 04/32] dt-bindings: Add root properties for Raspberry Pi 3 Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 05/32] ARM: bcm2835: Add devicetree for the " Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 06/32] arm64: Fix physical to DMA mappings Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-02 17:20 ` Catalin Marinas 2016-06-02 17:20 ` Catalin Marinas 2016-06-01 21:43 ` [PATCH 07/32] ARM: bcm2835: dt: Add the ethernet to the device trees Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 08/32] ARM: bcm2837: " Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 09/32] bcm2837-rpi-3-b.dts for 32bit arm Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 22:00 ` Arnd Bergmann 2016-06-01 22:00 ` Arnd Bergmann 2016-06-01 22:00 ` Arnd Bergmann 2016-06-01 22:30 ` Eric Anholt 2016-06-01 22:30 ` Eric Anholt 2016-06-01 22:30 ` Eric Anholt 2016-06-01 22:39 ` Arnd Bergmann 2016-06-01 22:39 ` Arnd Bergmann 2016-06-01 22:39 ` Arnd Bergmann 2016-06-02 6:56 ` Gerd Hoffmann 2016-06-02 6:56 ` Gerd Hoffmann 2016-06-02 6:56 ` Gerd Hoffmann 2016-06-02 7:24 ` Arnd Bergmann 2016-06-02 7:24 ` Arnd Bergmann 2016-06-02 7:24 ` Arnd Bergmann 2016-06-02 9:14 ` Gerd Hoffmann 2016-06-02 9:14 ` Gerd Hoffmann 2016-06-02 9:14 ` Gerd Hoffmann 2016-06-02 9:53 ` Arnd Bergmann 2016-06-02 9:53 ` Arnd Bergmann 2016-06-02 9:53 ` Arnd Bergmann 2016-06-02 15:11 ` Gerd Hoffmann 2016-06-02 15:11 ` Gerd Hoffmann 2016-06-02 15:11 ` Gerd Hoffmann 2016-06-02 15:23 ` Arnd Bergmann 2016-06-02 15:23 ` Arnd Bergmann 2016-06-02 15:23 ` Arnd Bergmann 2016-06-01 21:43 ` [PATCH 10/32] don't force serial pins to uart0 Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 22:19 ` Arnd Bergmann 2016-06-01 22:19 ` Arnd Bergmann 2016-06-01 22:19 ` Arnd Bergmann 2016-06-02 18:19 ` Eric Anholt 2016-06-02 18:19 ` Eric Anholt 2016-06-02 18:19 ` Eric Anholt 2016-06-01 21:43 ` [PATCH 11/32] ARM: bcm2835: Define standard pinctrl groups in the gpio node Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 12/32] ARM: bcm2835: Replace alt0/i2s_alt[02] with standard groups Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 13/32] bcm2837-rpi-3-b.dts: add gpio Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 14/32] bcm2837-rpi-3-b.dts: vc4 hdmi fixup Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 15/32] ARM: bcm2835: Move the emmc pin group to bcm283x.dtsi Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 16/32] bcm2837-rpi-3-b.dts: move the emmc pin group Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 17/32] ARM: bcm2835: Add a group for mapping pins 48-53 to sdhost Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 18/32] ARM: bcm2835: Move most RPi default pin groups to their devices Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 19/32] bcm2837-rpi-3-b.dts: move " Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 20/32] dt-bindings: Add binding for brcm,bcm2835-sdhost Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-06 13:12 ` Rob Herring 2016-06-06 13:12 ` Rob Herring 2016-06-06 13:12 ` Rob Herring 2016-06-01 21:43 ` [PATCH 21/32] mmc: bcm2835-sdhost: Add new driver for the internal SD controller Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 22:18 ` Arnd Bergmann [this message] 2016-06-01 22:18 ` Arnd Bergmann 2016-06-01 22:18 ` Arnd Bergmann 2016-06-02 18:12 ` Eric Anholt 2016-06-02 18:12 ` Eric Anholt 2016-06-02 18:12 ` Eric Anholt 2016-06-03 8:42 ` Arnd Bergmann 2016-06-03 8:42 ` Arnd Bergmann 2016-06-03 8:42 ` Arnd Bergmann 2016-06-21 12:33 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 22/32] ARM: bcm2835: Include SDHOST in the device tree Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 23/32] ARM: bcm2835: Enable SDHOST by default Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 24/32] i2c: bcm2835: Don't complain on -EPROBE_DEFER from getting our clock Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-07-22 7:26 ` Wolfram Sang 2016-07-22 7:26 ` Wolfram Sang 2016-07-22 7:26 ` Wolfram Sang 2016-06-01 21:43 ` [PATCH 25/32] dt-gpio-fix Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 26/32] enable uart1 on the BT pins Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 27/32] gpio: Add support for the FXL6408 GPIO expander Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann [not found] ` <1464817421-8519-28-git-send-email-kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-06-06 23:50 ` Eric Anholt 2016-06-06 23:50 ` Eric Anholt 2016-06-06 23:50 ` Eric Anholt 2016-06-07 0:02 ` Florian Fainelli 2016-06-07 0:02 ` Florian Fainelli 2016-06-07 0:02 ` Florian Fainelli 2016-06-08 9:10 ` Linus Walleij 2016-06-08 9:10 ` Linus Walleij 2016-06-08 9:10 ` Linus Walleij [not found] ` <CACRpkdax3AmnprT48zL7J92ddMvLYeRXi9OrQK_JrBT8_zZQoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-06-09 17:22 ` Eric Anholt 2016-06-09 17:22 ` Eric Anholt 2016-06-09 17:22 ` Eric Anholt 2016-06-01 21:43 ` [PATCH 28/32] i2c: bcm2835: Set up the rising/falling edge delays Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-07-22 7:26 ` Wolfram Sang 2016-07-22 7:26 ` Wolfram Sang 2016-07-22 7:26 ` Wolfram Sang 2016-06-01 21:43 ` [PATCH 29/32] ARM: bcm2835: Use i2c-gpio for the expander, instead Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 30/32] ARM: bcm2835: Add a new EMMC pin group from the downstream tree Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 31/32] enable wireless Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` [PATCH 32/32] mmc: bcm2835: Import bcm2835-mmc and switch to it Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-01 21:43 ` Gerd Hoffmann 2016-06-02 16:52 ` Stefan Wahren 2016-06-02 16:52 ` Stefan Wahren 2016-06-02 16:52 ` Stefan Wahren 2016-06-02 18:18 ` Eric Anholt 2016-06-02 18:18 ` Eric Anholt 2016-06-02 18:18 ` Eric Anholt 2016-06-02 19:09 ` Stefan Wahren 2016-06-02 19:09 ` Stefan Wahren 2016-06-02 19:09 ` Stefan Wahren 2016-06-02 20:03 ` Gerd Hoffmann 2016-06-02 20:03 ` Gerd Hoffmann 2016-06-02 20:03 ` Gerd Hoffmann 2016-06-02 21:40 ` Eric Anholt 2016-06-02 21:40 ` Eric Anholt 2016-06-02 21:40 ` Eric Anholt
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=4715245.LFBOAeJdHc@wuerfel \ --to=arnd@arndb.de \ --cc=eric@anholt.net \ --cc=kraxel@redhat.com \ --cc=lee@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mmc@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=swarren@wwwdotorg.org \ --cc=ulf.hansson@linaro.org \ /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: linkBe 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.