linux-kernel - Re: [PATCH 2/3] ARM: bus: da8xx-syscfg: new driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Oct 2016 21:05:50 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Kevin Hilman <khilman@...libre.com>
Cc:     Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Michael Turquette <mturquette@...libre.com>,
        Sekhar Nori <nsekhar@...com>, Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Mark Rutland <mark.rutland@....com>,
        Peter Ujfalusi <peter.ujfalusi@...com>,
        Russell King <linux@...linux.org.uk>,
        LKML <linux-kernel@...r.kernel.org>,
        arm-soc <linux-arm-kernel@...ts.infradead.org>,
        linux-drm <dri-devel@...ts.freedesktop.org>,
        linux-devicetree <devicetree@...r.kernel.org>,
        Jyri Sarha <jsarha@...com>,
        Tomi Valkeinen <tomi.valkeinen@...com>,
        David Airlie <airlied@...ux.ie>, Arnd Bergmann <arnd@...db.de>,
        Olof Johansson <olof@...om.net>
Subject: Re: [PATCH 2/3] ARM: bus: da8xx-syscfg: new driver

Hi Kevin,

On Thursday 20 Oct 2016 09:57:51 Kevin Hilman wrote:
> Laurent Pinchart <laurent.pinchart@...asonboard.com> writes:
> > On Wednesday 19 Oct 2016 10:26:57 Bartosz Golaszewski wrote:
> >> 2016-10-18 22:49 GMT+02:00 Laurent Pinchart:
> >>> On Monday 17 Oct 2016 18:30:49 Bartosz Golaszewski wrote:
> >>>> Create the driver for the da8xx System Configuration and implement
> >>>> support for writing to the three Master Priority registers.
> >>>> 
> >>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@...libre.com>
> >> 
> >> [snip]
> >> 
> >>>> +
> >>>> +Documentation:
> >>>> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
> >>>> +
> >>>> +Required properties:
> >>>> +
> >>>> +- compatible:                "ti,da850-syscfg"
> >>> 
> >>> Don't you need a reg property ?
> >> 
> >> Yes, Kevin already pointed that out. I'll add it in v2. Same for [1/3].
> >> 
> >>>> +Optional properties:
> >>>> +
> >>>> +The below properties are used to specify the priority of master
> >>>> peripherals.
> >>>> +They must be between 0-7 where 0 is the highest priority and 7 is the
> >>>> lowest.
> >>>> +
> >>>> +- ti,pri-arm-i:              ARM_I port priority.
> >>>> +
> >>>> +- ti,pri-arm-d:              ARM_D port priority.
> >>>> +
> >>>> +- ti,pri-upp:                uPP port priority.
> >>>> +
> >>>> +- ti,pri-sata:               SATA port priority.
> >>>> +
> >>>> +- ti,pri-pru0:               PRU0 port priority.
> >>>> +
> >>>> +- ti,pri-pru1:               PRU1 port priority.
> >>>> +
> >>>> +- ti,pri-edma30tc0:  EDMA3_0_TC0 port priority.
> >>>> +
> >>>> +- ti,pri-edma30tc1:  EDMA3_0_TC1 port priority.
> >>>> +
> >>>> +- ti,pri-edma31tc0:  EDMA3_1_TC0 port priority.
> >>>> +
> >>>> +- ti,pri-vpif-dma-0: VPIF DMA0 port priority.
> >>>> +
> >>>> +- ti,pri-vpif-dma-1: VPIF DMA1 port priority.
> >>>> +
> >>>> +- ti,pri-emac:               EMAC port priority.
> >>>> +
> >>>> +- ti,pri-usb0cfg:    USB0 CFG port priority.
> >>>> +
> >>>> +- ti,pri-usb0cdma:   USB0 CDMA port priority.
> >>>> +
> >>>> +- ti,pri-uhpi:               HPI port priority.
> >>>> +
> >>>> +- ti,pri-usb1:               USB1 port priority.
> >>>> +
> >>>> +- ti,pri-lcdc:               LCDC port priority.
> >>> 
> >>> I'm afraid this looks more like system configuration than hardware
> >>> description to me.
> >> 
> >> While you're certainly right, this approach is already implemented in
> >> several other memory and bus drivers and it was also suggested by
> >> Sekhar in one of the tilcdc rev1 threads. There's also no real
> >> alternative that I know of.
> > 
> > The fact that other drivers get it wrong is no excuse for copying them :-)
> 
> What exactly is "wrong" with the way other drivers are doing it?
> 
> I'm sure there may be other ideas, and possibly some better ones, but
> that doesn't make it wrong, and doesn't change he fact that the kernel
> has existing drivers SoC-bus-specific system performance knobs like
> this.

It's not the drivers I'm concerned about, but the DT bindings. The proposed DT 
binding contains a large number of properties that don't describe the hardware 
but contain configuration data. If they're accepted you'll have to carry them 
forward forever, while they should be controlled in a more flexible way.

> >>> There was a BoF session about how to support this kind of performance
> >>> knobs at ELCE last week:
> >>> https://openiotelceurope2016.sched.org/event/7rss/bof-linux-device-perf
> >>> ormance-framework-michael-turquette-baylibre :-)
> >> 
> >> Unfortunately it was just a discussion about potential approaches -
> >> there's no code yet.
> > 
> > Patches are welcome ;-)
> 
> Any generic perf framework will have to build on the HW-specifics of
> individual busses, so IMO, the lack of a generic performance
> framework/knobs should not be a reason to block the inclusion of any
> bus-specific knobs.
> 
> I guess this ultimately would go though arm-soc, so I've added Arnd &
> Olof to the thread.

-- 
Regards,

Laurent Pinchart