Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1330093

Re: [PATCH 1/3] input: cygnus-update touchscreen dt node document

Show all headers | View raw

On Tuesday 09 February 2016 14:27:55 Raveendra Padasalagi wrote:
> +
> +One of the below 2 property is mandatory.
> +- ts_syscon: handler of syscon node defining physical base address
> +  of the controller and length of memory mapped region.
> +  If this property is selected please make sure MFD_SYSCON config
> +  is enabled in the defconfig file.
> +or
> +- reg: physical base address of the controller and length of memory
> +  mapped region.
> +
>  - clocks:  The clock provided by the SOC to driver the tsc
>  - clock-name:  name for the clock
>  - interrupts: The touchscreen controller's interrupt
> @@ -54,12 +62,16 @@ Optional properties:
>  - touchscreen-inverted-y: Y axis is inverted (boolean)
>  
>  Example:
> +       ts_adc_syscon: ts_adc_syscon@0x180a6000 {
> +               compatible = "syscon";
> +               reg = <0x180a6000 0xc30>;
> +       };
>  
>         touchscreen: tsc@0x180A6000 {
>                 compatible = "brcm,iproc-touchscreen";
>                 #address-cells = <1>;
>                 #size-cells = <1>;
> -               reg = <0x180A6000 0x40>;
> +               syscon = <&ts_adc_syscon>;
>                 clocks = <&adc_clk>;
>                 clock-names = "tsc_clk";
>                 interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;

This doesn't seem right: For one thing, it's an incompatible binding
change which you should try to avoid.

Worse, the syscon has a name indicating that it really has a particular
function, but the name is completely generic (just "syscon"). Please
add a specific compatible string identifying the device that owns
the registers, and put any other associated properties (clocks,
interrupts, ...) that are for that device in general rather than just
the touchscreen in there.

Your changelog text fails to describe why you do this change in
the first place, and why you can't do it in a backward-compatible
way, so please rewrite that text.

	Arnd

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/3] input: cygnus-update touchscreen dt node document Raveendra Padasalagi <raveendra.padasalagi@broadcom.com> - 2016-02-09 10:10 +0100
  Re: [PATCH 1/3] input: cygnus-update touchscreen dt node document Arnd Bergmann <arnd@arndb.de> - 2016-02-09 12:10 +0100
    RE: [PATCH 1/3] input: cygnus-update touchscreen dt node document Raveendra Padasalagi <raveendra.padasalagi@broadcom.com> - 2016-02-09 18:10 +0100
      Re: [PATCH 1/3] input: cygnus-update touchscreen dt node document Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-02-09 18:50 +0100
        RE: [PATCH 1/3] input: cygnus-update touchscreen dt node document Raveendra Padasalagi <raveendra.padasalagi@broadcom.com> - 2016-02-10 06:00 +0100
          Re: [PATCH 1/3] input: cygnus-update touchscreen dt node document Rob Herring <robh@kernel.org> - 2016-02-12 16:10 +0100
            RE: [PATCH 1/3] input: cygnus-update touchscreen dt node document Raveendra Padasalagi <raveendra.padasalagi@broadcom.com> - 2016-02-16 05:30 +0100

csiph-web