| Message ID | 3935244.Dm2HkZj2Bg@wuerfel (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Arnd, On Fri, 28 Nov 2014 00:12:25 +0100 Arnd Bergmann <arnd@arndb.de> wrote: > On Thursday 27 November 2014 18:12:43 Alexandre Belloni wrote: > > > > As discussed some weeks ago, I prepared patches to switch sama5d[3-4] to > > multiplatform. We are still missing the SMC and matrix drivers to switch > > sam9 and rm9200. > > I just looked at the drivers because I got curious, and to see if > there are still any low-hanging fruit, but I guess you already picked > them all ;-) > > > The currently affected drivers are: > > - drivers/ata/pata_at91.c (SMC) > > - drivers/pcmcia/at91_cf.c (SMC) > > I guess the SMC should live in drivers/memory with an interface > similar to mvebu-devbus.c? Actually, there's some work in progress to support the EBI/SMC blocks as a memory controller driver ;-): http://thread.gmane.org/gmane.linux.kernel/1822096 I'll post a new version soon. > > Seems doable but nontrivial. > > > - drivers/usb/gadget/udc/at91_udc.c (Matrix, this is the only one > > for sam9) > > Is at91_matrix a pin controller? With the board files removed, the udc > driver has the only two remaining calls to at91_matrix_{read,write} > for setting the pullup, so that could be modeled as a trivial pinctrl > driver The matrix block is containing several system configuration registers. Most of them are related to AHB/APB bus config (master <-> slave priority, burst and some other configs I don't remember). Another register is here to define which HW logic is attached to an external device connected through the EBI (External Bus Interface): NAND, SDRAM, CompactFlash, ... And, as you pointed out, there's a register to configure the pullup of the UDC device. As you can see, the matrix registers might be accessed by different drivers (include the EBI/SMC driver), hence I proposed to expose them as a syscon device (see the EBI/SMC series). Best Regards, Boris
On Friday 28 November 2014 00:39:38 Boris Brezillon wrote: > Hi Arnd, > > On Fri, 28 Nov 2014 00:12:25 +0100 > Arnd Bergmann <arnd@arndb.de> wrote: > > > On Thursday 27 November 2014 18:12:43 Alexandre Belloni wrote: > > > > > > As discussed some weeks ago, I prepared patches to switch sama5d[3-4] to > > > multiplatform. We are still missing the SMC and matrix drivers to switch > > > sam9 and rm9200. > > > > I just looked at the drivers because I got curious, and to see if > > there are still any low-hanging fruit, but I guess you already picked > > them all > > > > > The currently affected drivers are: > > > - drivers/ata/pata_at91.c (SMC) > > > - drivers/pcmcia/at91_cf.c (SMC) > > > > I guess the SMC should live in drivers/memory with an interface > > similar to mvebu-devbus.c? > > Actually, there's some work in progress to support the EBI/SMC blocks > as a memory controller driver ;-): > > http://thread.gmane.org/gmane.linux.kernel/1822096 > > I'll post a new version soon. Ok, cool. > > > > Seems doable but nontrivial. > > > > > - drivers/usb/gadget/udc/at91_udc.c (Matrix, this is the only one > > > for sam9) > > > > Is at91_matrix a pin controller? With the board files removed, the udc > > driver has the only two remaining calls to at91_matrix_{read,write} > > for setting the pullup, so that could be modeled as a trivial pinctrl > > driver > > The matrix block is containing several system configuration registers. > Most of them are related to AHB/APB bus config (master <-> slave > priority, burst and some other configs I don't remember). > Another register is here to define which HW logic is attached to an > external device connected through the EBI (External Bus Interface): > NAND, SDRAM, CompactFlash, ... > And, as you pointed out, there's a register to configure the pullup of > the UDC device. > > As you can see, the matrix registers might be accessed by different > drivers (include the EBI/SMC driver), hence I proposed to expose them as > a syscon device (see the EBI/SMC series). > Sounds good. Arnd
On 28/11/2014 at 00:12:25 +0100, Arnd Bergmann wrote : > On Thursday 27 November 2014 18:12:43 Alexandre Belloni wrote: > > > > As discussed some weeks ago, I prepared patches to switch sama5d[3-4] to > > multiplatform. We are still missing the SMC and matrix drivers to switch > > sam9 and rm9200. > > I just looked at the drivers because I got curious, and to see if > there are still any low-hanging fruit, but I guess you already picked > them all ;-) Yeah, my main goal was to test multiplatform on sama5d3 and it was actually quite easy. The worst still being rm9200 ;) > > > - sound/atmel/ac97c.c (that one is still not converted to DT anyway...) > > This one seems fairly straightforward to do, including a DT binding, > but the result is still ugly as it supports the at32 chips that > do things very differently. > > The patch below gets it to compile and should be enough as a replacement > once a compatible string gets added. > Actually, some work was done but we never saw an other version of the series. Alexander, are you still interested? Else we can take the patch below. http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/246679.html > > - drivers/watchdog/at91rm9200_wdt.c (WIP, will be converted properly to > > an MFD) > > I think we discussed this one before. Remind me why we can't just convert > it to use watchdog_register() for simplification and then move whatever > remains into the arch/arm/mach-at91/at91rm9200_time.c file, or both > into drivers/clocksource. > Yeah, the new plan is to merge the watchdog with at91rm9200_time.c and move it either to drivers/clocksource or make an mfd. I believe the former would be easier. > Arnd > > 8<---- > ASoC: atmel/ac97c: remove platform_data dependency > > As at91 gets changed to multiplatform, we can't use the mach/cpu.h > header any more, but this is ok as it only gets used to check for > cpu_is_at32ap7000(), which arch/avr32. > > In order to make the driver work without platform_data, this also > changes the reset gpio line handling so it can look up the gpio > descriptor from DT. It is still missing a compatible string and > a binding that describes the valid DT properties. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c > index b59427d5a697..4eec216b7f92 100644 > --- a/sound/atmel/ac97c.c > +++ b/sound/atmel/ac97c.c > @@ -34,10 +34,10 @@ > #include <linux/platform_data/dma-dw.h> > #include <linux/dma/dw.h> > > +#ifdef CONFIG_AVR32 > #include <mach/cpu.h> > - > -#ifdef CONFIG_ARCH_AT91 > -#include <mach/hardware.h> > +#else > +#define cpu_is_at32ap7000() (0) > #endif > > #include "ac97c.h" > @@ -78,7 +78,7 @@ struct atmel_ac97c { > void __iomem *regs; > int irq; > int opened; > - int reset_pin; > + struct gpio_desc *reset_pin; > }; > > #define get_chip(card) ((struct atmel_ac97c *)(card)->private_data) > @@ -890,11 +890,11 @@ static void atmel_ac97c_reset(struct atmel_ac97c *chip) > ac97c_writel(chip, CAMR, 0); > ac97c_writel(chip, COMR, 0); > > - if (gpio_is_valid(chip->reset_pin)) { > - gpio_set_value(chip->reset_pin, 0); > + if (chip->reset_pin) { > + gpiod_set_value(chip->reset_pin, 0); > /* AC97 v2.2 specifications says minimum 1 us. */ > udelay(2); > - gpio_set_value(chip->reset_pin, 1); > + gpiod_set_value(chip->reset_pin, 1); > } else { > ac97c_writel(chip, MR, AC97C_MR_WRST | AC97C_MR_ENA); > udelay(2); > @@ -923,7 +923,7 @@ static int atmel_ac97c_probe(struct platform_device *pdev) > } > > pdata = pdev->dev.platform_data; > - if (!pdata) { > + if (cpu_is_at32ap7000() && !pdata) { > dev_dbg(&pdev->dev, "no platform data\n"); > return -ENXIO; > } > @@ -980,16 +980,18 @@ static int atmel_ac97c_probe(struct platform_device *pdev) > goto err_ioremap; > } > > - if (gpio_is_valid(pdata->reset_pin)) { > - if (gpio_request(pdata->reset_pin, "reset_pin")) { > + if (pdata && gpio_is_valid(pdata->reset_pin)) { > + if (devm_gpio_request(&pdev->dev, pdata->reset_pin, "reset")) { > dev_dbg(&pdev->dev, "reset pin not available\n"); > - chip->reset_pin = -ENODEV; > + chip->reset_pin = NULL; > } else { > gpio_direction_output(pdata->reset_pin, 1); > - chip->reset_pin = pdata->reset_pin; > + chip->reset_pin = gpio_to_desc(pdata->reset_pin); > } > } else { > - chip->reset_pin = -EINVAL; > + chip->reset_pin = devm_gpiod_get(&pdev->dev, "reset", 0); > + if (IS_ERR(chip->reset_pin)) > + chip->reset_pin = NULL; > } > > atmel_ac97c_reset(chip); > @@ -1113,9 +1115,6 @@ err_dma: > chip->dma.tx_chan = NULL; > } > err_ac97_bus: > - if (gpio_is_valid(chip->reset_pin)) > - gpio_free(chip->reset_pin); > - > iounmap(chip->regs); > err_ioremap: > free_irq(irq, chip); > @@ -1170,9 +1169,6 @@ static int atmel_ac97c_remove(struct platform_device *pdev) > struct snd_card *card = platform_get_drvdata(pdev); > struct atmel_ac97c *chip = get_chip(card); > > - if (gpio_is_valid(chip->reset_pin)) > - gpio_free(chip->reset_pin); > - > ac97c_writel(chip, CAMR, 0); > ac97c_writel(chip, COMR, 0); > ac97c_writel(chip, MR, 0); >
Hi Alexandre, On Friday 28 November 2014, 01:28:22 wrote Alexandre Belloni: > > > - sound/atmel/ac97c.c (that one is still not converted to DT anyway...) > > > > This one seems fairly straightforward to do, including a DT binding, > > but the result is still ugly as it supports the at32 chips that > > do things very differently. > > > > The patch below gets it to compile and should be enough as a replacement > > once a compatible string gets added. > > > > Actually, some work was done but we never saw an other version of the > series. Alexander, are you still interested? Else we can take the patch > below. > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/246679.html Well, I tried to get a ac97c driver based on alsa soc framework as suggested by Takashi. But I failed so far, I could not get my head around those architecture. When keeping my DT support on the current driver, the only thing which is open that the bindings should be modifed that they are like soc-ac97link bindings. I could work on that, but I currently don't know when I can get access again to that hardware for testing. Best regards, Alexander
diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c index b59427d5a697..4eec216b7f92 100644 --- a/sound/atmel/ac97c.c +++ b/sound/atmel/ac97c.c @@ -34,10 +34,10 @@ #include <linux/platform_data/dma-dw.h> #include <linux/dma/dw.h> +#ifdef CONFIG_AVR32 #include <mach/cpu.h> - -#ifdef CONFIG_ARCH_AT91 -#include <mach/hardware.h> +#else +#define cpu_is_at32ap7000() (0) #endif #include "ac97c.h" @@ -78,7 +78,7 @@ struct atmel_ac97c { void __iomem *regs; int irq; int opened; - int reset_pin; + struct gpio_desc *reset_pin; }; #define get_chip(card) ((struct atmel_ac97c *)(card)->private_data) @@ -890,11 +890,11 @@ static void atmel_ac97c_reset(struct atmel_ac97c *chip) ac97c_writel(chip, CAMR, 0); ac97c_writel(chip, COMR, 0); - if (gpio_is_valid(chip->reset_pin)) { - gpio_set_value(chip->reset_pin, 0); + if (chip->reset_pin) { + gpiod_set_value(chip->reset_pin, 0); /* AC97 v2.2 specifications says minimum 1 us. */ udelay(2); - gpio_set_value(chip->reset_pin, 1); + gpiod_set_value(chip->reset_pin, 1); } else { ac97c_writel(chip, MR, AC97C_MR_WRST | AC97C_MR_ENA); udelay(2); @@ -923,7 +923,7 @@ static int atmel_ac97c_probe(struct platform_device *pdev) } pdata = pdev->dev.platform_data; - if (!pdata) { + if (cpu_is_at32ap7000() && !pdata) { dev_dbg(&pdev->dev, "no platform data\n"); return -ENXIO; } @@ -980,16 +980,18 @@ static int atmel_ac97c_probe(struct platform_device *pdev) goto err_ioremap; } - if (gpio_is_valid(pdata->reset_pin)) { - if (gpio_request(pdata->reset_pin, "reset_pin")) { + if (pdata && gpio_is_valid(pdata->reset_pin)) { + if (devm_gpio_request(&pdev->dev, pdata->reset_pin, "reset")) { dev_dbg(&pdev->dev, "reset pin not available\n"); - chip->reset_pin = -ENODEV; + chip->reset_pin = NULL; } else { gpio_direction_output(pdata->reset_pin, 1); - chip->reset_pin = pdata->reset_pin; + chip->reset_pin = gpio_to_desc(pdata->reset_pin); } } else { - chip->reset_pin = -EINVAL; + chip->reset_pin = devm_gpiod_get(&pdev->dev, "reset", 0); + if (IS_ERR(chip->reset_pin)) + chip->reset_pin = NULL; } atmel_ac97c_reset(chip); @@ -1113,9 +1115,6 @@ err_dma: chip->dma.tx_chan = NULL; } err_ac97_bus: - if (gpio_is_valid(chip->reset_pin)) - gpio_free(chip->reset_pin); - iounmap(chip->regs); err_ioremap: free_irq(irq, chip); @@ -1170,9 +1169,6 @@ static int atmel_ac97c_remove(struct platform_device *pdev) struct snd_card *card = platform_get_drvdata(pdev); struct atmel_ac97c *chip = get_chip(card); - if (gpio_is_valid(chip->reset_pin)) - gpio_free(chip->reset_pin); - ac97c_writel(chip, CAMR, 0); ac97c_writel(chip, COMR, 0); ac97c_writel(chip, MR, 0);