Tony and Arnd Thanks for the comments On 04/29/2014 07:22 PM, Tony Lindgren wrote: > * Arnd Bergmann <arnd@xxxxxxxx> [140429 13:35]: >> On Tuesday 29 April 2014 15:19:47 Dan Murphy wrote: >>> + * AM33xx reset index for PRCM Module >>> + * >>> + * Copyright 2014 Texas Instruments Inc. >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + */ >>> + >>> +#ifndef _DT_BINDINGS_RESET_TI_AM33XX_H >>> +#define _DT_BINDINGS_RESET_TI_AM33XX_H >>> + >>> +#define RESET_DEVICE_RESET 0 >>> +#define RESET_GFX_RESET 1 >>> +#define RESET_PER_RESET 2 >>> + >>> +#endif >> Interfaces like this should only be used if you can't use hardware >> numbers, in general. If these numbers are in the data sheet, just >> put them directly into the dts file, as we do for interrupt numbers, >> gpio numbers, register offsets etc. >> >> If you have made them up to define an interface between the driver >> and DT because there is no usable hardare ID, I'd suggest just using >> a single file across all SoCs that have this driver, and have >> a unified name space. > Also, it's a bit unclear how the reset controller phandle is used > referenced and used by the consumer device.. Maybe setting that up > first in a Linux generic way is a good point starting point. > > Maybe something like this along the same way as clocks are set up > (completely untested): > > &reset1 { > iva_reset: reset1 { > reg = /bits/ 8 <0>; > }; > gfx_reset: reset1 { > reg = /bits/ 8 <1>; > }; > ... > }; > > &iva { > compatible = "ti,ivahd"; > resets = <&reset1 1>; > ... > }; I had something very similar to this when I was developing this driver but moved away from this. Following the clocks implementation I had a separate dtsi for resets for each device and had the data defined like so for each SoC. &prcm_resets { device_reset: device_reset { rstctrl_offs = <0x1104>; ctrl_bit-shift = <0>; rstst_offs = <0x1114>; sts_bit-shift = <0>; }; gpu_reset: gpu_reset { rstctrl_offs = <0x0D00>; ctrl_bit-shift = <3>; rstst_offs = <0x0D0C>; sts_bit-shift = <5>; }; }; And then any client interested in a specific reset driver would include this resets = <&prcm_resets &gpu_reset>; reset-names = "gpu_reset"; Our reset code would then retrieve the register data through the phandle instead of an index. Thoughts? Dan > Regards, > > Tony -- ------------------ Dan Murphy -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html