Re: [PATCH] clk: renesas: r8a7796: Add DRIF clock

[Geert Uytterhoeven]

On Thu, Oct 13, 2016 at 11:31 AM, Ramesh Shanmugasundaram
 wrote:
> This patch adds DRIF module clocks for r8a7796 SoC.
>
> Signed-off-by: Ramesh Shanmugasundaram 
> 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

RE: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port

[Chris Brandt]

On 9/22/2016, Ulf Hansson wrote:
> To: Chris Brandt 
> Cc: Wolfram Sang ; Sergei Shtylyov
> ; Geert Uytterhoeven  m68k.org>; Simon Horman ; linux-mmc  m...@vger.kernel.org>; Linux-Renesas ;
> Lee Jones 
> Subject: Re: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port
> 
> + Lee
> 
> On 12 September 2016 at 16:15, Chris Brandt 
> wrote:
> > For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.
> > Therefore a new flag has been created that will allow 32-bit
> > reads/writes to the DATA_PORT register instead of 16-bit (because
> > 16-bits accesses are not supported).
> >
> > Signed-off-by: Chris Brandt 
> > ---
> > v3:
> > * changed loops to memcpy
> > v2:
> > * changed 'data * 0xFF' to 'data & 0xFF'
> > * added 'const' for sd_ctrl_write32_rep
> > ---
> >  drivers/mmc/host/tmio_mmc.h | 12 
> >  drivers/mmc/host/tmio_mmc_pio.c | 30 ++
> >  include/linux/mfd/tmio.h|  5 +
> 
> This header file needs to be split up. The mmc specific bits, should be
> moved to a local header file under driver/mmc/host/*.
> 
> Sure, we don't need to do that as part of $subject patch, but then I need
> an ack from Lee Jones, the mfd maintainer. I have added him on cc.


Is this still waiting on an ACK from Lee Jones because it touches the tmio.h 
file under include/linux/mfd/ ?





 
> >  3 files changed, 47 insertions(+)
> >
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index ecb99fc..a99e634 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -237,6 +237,12 @@ static inline u32
> sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host, int ad
> >readw(host->ctl + ((addr + 2) << host->bus_shift)) <<
> > 16;  }
> >
> > +static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int
> addr,
> > +   u32 *buf, int count)
> > +{
> > +   readsl(host->ctl + (addr << host->bus_shift), buf, count); }
> > +
> >  static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int
> > addr, u16 val)  {
> > /* If there is a hook and it returns non-zero then there @@
> > -259,4 +265,10 @@ static inline void sd_ctrl_write32_as_16_and_16(struct
> tmio_mmc_host *host, int
> > writew(val >> 16, host->ctl + ((addr + 2) <<
> > host->bus_shift));  }
> >
> > +static inline void sd_ctrl_write32_rep(struct tmio_mmc_host *host, int
> addr,
> > +   const u32 *buf, int count) {
> > +   writesl(host->ctl + (addr << host->bus_shift), buf, count); }
> > +
> >  #endif
> > diff --git a/drivers/mmc/host/tmio_mmc_pio.c
> > b/drivers/mmc/host/tmio_mmc_pio.c index 017a4dc..59fac7d 100644
> > --- a/drivers/mmc/host/tmio_mmc_pio.c
> > +++ b/drivers/mmc/host/tmio_mmc_pio.c
> > @@ -443,6 +443,36 @@ static void tmio_mmc_transfer_data(struct
> tmio_mmc_host *host,
> > /*
> >  * Transfer the data
> >  */
> > +   if (host->pdata->flags & TMIO_MMC_32BIT_DATA_PORT) {
> > +   u8 data[4] = { };
> > +
> > +   if (is_read)
> > +   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, (u32
> *)buf,
> > +  count >> 2);
> > +   else
> > +   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, (u32
> *)buf,
> > +   count >> 2);
> > +
> > +   /* if count was multiple of 4 */
> > +   if (!(count & 0x3))
> > +   return;
> > +
> > +   buf8 = (u8 *)(buf + (count >> 2));
> > +   count %= 4;
> > +
> > +   if (is_read) {
> > +   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT,
> > +  (u32 *)data, 1);
> > +   memcpy(buf8, data, count);
> > +   } else {
> > +   memcpy(data, buf8, count);
> > +   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT,
> > +   (u32 *)data, 1);
> > +   }
> > +
> > +   return;
> > +   }
> > +
> > if (is_read)
> > sd_ctrl_read16_rep(host, CTL_SD_DATA_PORT, buf, count >>
> 1);
> > else
> > diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h index
> > 3b95dc7..0dbcb7e 100644
> > --- a/include/linux/mfd/tmio.h
> > +++ b/include/linux/mfd/tmio.h
> > @@ -100,6 +100,11 @@
> >  #define TMIO_MMC_SDIO_STATUS_QUIRK (1 << 8)
> >
> >  /*
> > + * Some controllers have a 32-bit wide data port register  */
> > +#define TMIO_MMC_32BIT_DATA_PORT   (1 << 9)
> > +
> > +/*
> >   * Some controllers allows to set SDx actual clock
> >   */
> >  #define 

RE: [PATCH v3 0/3] mmc: sh_mobile_sdhi: Add r7s72100 support

[Chris Brandt]

Hello.

What ever happened to this series of patches?

They didn't seem too outrageous, but they never got applied.

Thank you.

Chris



On 9/12/2016, Chris Brandt wrote:
> To: Ulf Hansson ; Wolfram Sang  engineering.com>; Sergei Shtylyov ;
> Geert Uytterhoeven 
> Cc: Simon Horman ; linux-...@vger.kernel.org;
> linux-renesas-soc@vger.kernel.org; Chris Brandt 
> Subject: [PATCH v3 0/3] mmc: sh_mobile_sdhi: Add r7s72100 support
> 
> For the most part, the SDHI controller in the RZ/A1 (r7s72100) is the same
> as other Renesas SoC...except for the fact that the data port register was
> widened to 32-bits and the 16-bit accesses in the current tmio driver
> aren't going to cut it.
> 
> Also, the tmio driver allows you to pass in what voltages you support via
> the ocr_mask. For the RZ/A1, it's just simple 3.3v.
> So, I added the ability to pass that when using DT.
> 
> Chris Brandt (3):
>   mmc: sh_mobile_sdhi: add ocr_mask option
>   mmc: tmio-mmc: add support for 32bit data port
>   mmc: sh_mobile_sdhi: Add r7s72100 support
> 
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt |  1 +
>  drivers/mmc/host/sh_mobile_sdhi.c  |  9 +++
>  drivers/mmc/host/tmio_mmc.h| 12 +
>  drivers/mmc/host/tmio_mmc_pio.c| 30
> ++
>  include/linux/mfd/tmio.h   |  5 
>  5 files changed, 57 insertions(+)
> 
> --
> 2.9.2
> 

Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels

[Rob Herring]

On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> > LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> > Multiple incompatible data link layers have been used over time to
>> > transmit image data to LVDS panels. This binding supports display panels
>> > compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> > specifications.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > 
>> > ---
>> >
>> >  .../bindings/display/panel/panel-lvds.txt  | 119 
>> >  1 file changed, 119 insertions(+)
>> >  create mode 100644
>> >  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
>> > diff --git
>> > a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> > b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt new file
>> > mode 100644
>> > index ..250861f2673e
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> > @@ -0,0 +1,119 @@
>> > +Generic LVDS Panel
>> > +==
>> > +
>> > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> > Multiple
>> > +incompatible data link layers have been used over time to transmit image
>> > data
>> > +to LVDS panels. This bindings supports display panels compatible with the
>> > +following specifications.
>> > +
>> > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
>> > February
>> > +1999 (Version 1.0), Japan Electronic Industry Development Association
>> > (JEIDA)
>> > +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
>> > +Semiconductor
>> > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
>> > +Electronics Standards Association (VESA)
>> > +
>> > +Device compatible with those specifications have been marketed under the
>> > +FPD-Link and FlatLink brands.
>> > +
>> > +
>> > +Required properties:
>> > +- compatible: shall contain "panel-lvds"
>>
>> Maybe as a fallback, but on its own, no way.
>
> Which brings an interesting question: when designing generic DT bindings,
> what's the rule regarding

Call it "simple" so I can easily NAK it. :)

Define a generic structure, not a single binding trying to serve all.

>
>> > +- width-mm: panel display width in millimeters
>> > +- height-mm: panel display height in millimeters
>>
>> This is already documented for all panels IIRC.
>
> Note that this DT binding has nothing to do with the simple-panel binding. It
> is instead similar to the panel-dpi and panel-dsi-cm bindings (which currently
> don't but should specify the panel size in DT). The LVDS panel driver will
> *not* include any panel-specific information such as size or timings, these
> are specified in DT.

The panel bindings aren't really different. The biggest difference was
location in the tree, but we now generally allow panels to be either a
child of the LCD controller or connected with OF graph. We probably
need to work on restructuring the panel bindings a bit. We should have
an inheritance with a base panel binding of things like size, label,
graph, backlight, etc, then perhaps an interface specific bindings for
LVDS, DSI, and parallel, then a panel specific binding. With this the
panel specific binding is typically just a compatible string and which
inherited properties apply to it.


>> > +- data-mapping: the color signals mapping order, "jeida-18", "jeida-24"
>> > +  or "vesa-24"
>>
>> Maybe this should be part of the compatible.
>
> I've thought about it, but given that some panels support selecting between
> multiple modes (through a mode pin that is usually hardwired), I believe a
> separate DT property makes sense.

Okay.

> Furthermore, LVDS data organization is controlled by the combination of both
> data-mapping and data-mirror. It makes little sense from my point of view to
> handle one as part of the compatible string and the other one as a separate
> property.
>
>> > +Optional properties:
>> > +- label: a symbolic name for the panel
>>
>> Could be for any panel or display connector.
>
> Yes, but I'm not sure to understand how that's relevant :-)

Meaning it should be a common property.

>> > +- avdd-supply: reference to the regulator that powers the panel
>> analog supply
>> > +- dvdd-supply: reference to the regulator that powers the panel digital
>> > supply
>>
>> Which one has to be powered on first, what voltage, and with what time
>> in between? This is why "generic" or "simple" bindings don't work.
>
> The above-mentioned specifications also define connectors, pinouts and power
> supplies, but many LVDS panels compatible with the LVDS physical and data
> layers use a different connector with small differences in power supplies.
>
> I believe the voltage is irrelevant here, it doesn't need to be