linux-kernel - Re: [PATCH 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC 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]

Message-ID: <3899885.YaIxR1zelv@wuerfel>
Date:	Mon, 08 Sep 2014 12:19:05 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Stanimir Varbanov <svarbanov@...sol.com>
Cc:	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Pawel Moll <pawel.moll@....com>,
	Rob Herring <robh+dt@...nel.org>,
	Kumar Gala <galak@...eaurora.org>,
	Mark Rutland <mark.rutland@....com>,
	Grant Likely <grant.likely@...aro.org>,
	Jonathan Cameron <jic23@...nel.org>,
	linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Hartmut Knaack <knaack.h@....de>,
	Angelo Compagnucci <angelo.compagnucci@...il.com>,
	Doug Anderson <dianders@...omium.org>,
	Fugang Duan <B38611@...escale.com>,
	Johannes Thumshirn <johannes.thumshirn@....de>,
	Jean Delvare <jdelvare@...e.de>,
	Philippe Reynes <tremyfr@...oo.fr>,
	Lee Jones <lee.jones@...aro.org>,
	Josh Cartwright <joshc@...eaurora.org>,
	Stephen Boyd <sboyd@...eaurora.org>,
	David Collins <collinsd@...eaurora.org>,
	"Ivan T. Ivanov" <iivanov@...sol.com>
Subject: Re: [PATCH 1/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver

On Monday 08 September 2014 11:13:50 Stanimir Varbanov wrote:
> On 09/05/2014 03:26 PM, Arnd Bergmann wrote:
> > On Friday 05 September 2014 15:14:33 Stanimir Varbanov wrote:
> >> +       VADC_CHAN(LR_MUX1_BAT_THERM, 0)                 /* 0x30 */
> >> +       VADC_CHAN(LR_MUX2_BAT_ID, 0)
> >> +       VADC_CHAN(LR_MUX3_XO_THERM, 0)
> >> +       VADC_CHAN(LR_MUX4_AMUX_THM1, 0)
> >> +       VADC_CHAN(LR_MUX5_AMUX_THM2,by you  0)
> >> +       VADC_CHAN(LR_MUX6_AMUX_THM3, 0)
> >> +       VADC_CHAN(LR_MUX7_HW_ID, 0)
> >> +       VADC_CHAN(LR_MUX8_AMUX_THM4, 0)
> >> +       VADC_CHAN(LR_MUX9_AMUX_THM5, 0)
> >> +       VADC_CHAN(AMUX_PU1, 0)
> >> +       VADC_CHAN(AMUX_PU2, 0)
> >> +       VADC_CHAN(LR_MUX3_BUF_XO_THERM_BUF, 0)          /* 0x3c */
> >> +
> >> +       VADC_CHAN(LR_MUX1_PU1_BAT_THERM, 0)             /* 0x70 */
> >> +       VADC_CHAN(LR_MUX2_PU1_BAT_ID, 0)
> >> +       VADC_CHAN(LR_MUX3_PU1_XO_THERM, 0)
> >> +       VADC_CHAN(LR_MUX4_PU1_AMUX_THM1, 0)
> >> +       VADC_CHAN(LR_MUX5_PU1_AMUX_THM2, 0)
> >> +       VADC_CHAN(LR_MUX6_PU1_AMUX_THM3, 0)
> >> +       VADC_CHAN(LR_MUX7_PU1_AMUX_HW_ID, 0)
> >> +       VADC_CHAN(LR_MUX8_PU1_AMUX_THM4, 0)
> >> +       VADC_CHAN(LR_MUX9_PU1_AMUX_THM5, 0)
> >> +       VADC_CHAN(LR_MUX10_PU1_AMUX_USB_ID, 0)          /* 0x79 */
> >> +       VADC_CHAN(LR_MUX3_BUF_PU1_XO_THERM_BUF, 0)      /* 0x7c */
> >>
> > 
> > These numbers all look hardware specific, so why put macros into the
> > device tree rather than using them directly?
> 
> The idea was to use #defines in DT nodes when we need to overwrite the
> adc channel parameters, see example in 2/2 how it will be used.

I don't understand. The node in the example has

+               /* Channel node */
+               usb_id_nopull@39 {
+                       qcom,channel = <VADC_LR_MUX10_USB_ID>;
...
+               };


And VADC_LR_MUX10_USB_ID is defined to 0x39.  How is this helping anything?
You just introduce an artificial dependency on the header file, which makes
it a mess to merge the patches or do updates, and anybody who needs to
make updates to this now has to go through the same pain, to update the
dts files, the driver and the binding document in lockstep.

Why not remove the qcom,channel property completely and use a 'reg'
property with #address-cells=<1>, #size-cells=<0> and put the number
directly in there, with no need for obfuscation macros?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/