linux-kernel - Re: [PATCH v2 1/2] ARM: dts: imx6: Add support for Toradex Apalis iMX6Q/D SoM

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: <20160108084247.GB27359@ibawizard.net>
Date:	Fri, 8 Jan 2016 09:42:47 +0100
From:	Petr Štetiar <ynezz@...e.cz>
To:	Marcel Ziswiler <marcel.ziswiler@...adex.com>
Cc:	"ynezz@...e.cz" <ynezz@...e.cz>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	"pawel.moll@....com" <pawel.moll@....com>,
	"stefan@...er.ch" <stefan@...er.ch>,
	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"festevam@...il.com" <festevam@...il.com>,
	"mark.rutland@....com" <mark.rutland@....com>,
	"stillcompiling@...il.com" <stillcompiling@...il.com>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	"galak@...eaurora.org" <galak@...eaurora.org>,
	"shawnguo@...nel.org" <shawnguo@...nel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"l.stach@...gutronix.de" <l.stach@...gutronix.de>,
	"kernel@...gutronix.de" <kernel@...gutronix.de>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>
Subject: Re: [PATCH v2 1/2] ARM: dts: imx6: Add support for Toradex Apalis
	iMX6Q/D SoM

Marcel Ziswiler <marcel.ziswiler@...adex.com> [2016-01-06 09:15:18]:

> > > <&pinctrl_regulator_usbotg_pwr>;
> > > +			regulator-name = "usb_otg_vbus";
> > > +			regulator-min-microvolt = <5000000>;
> > > +			regulator-max-microvolt = <5000000>;
> > > +			gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
> > > +			enable-active-high;
> > > +			status = "disabled";
> > > +		};
> > 
> > I'm not sure, but it seems to me, that we should move this regulator into
> > the carrier board DTS. On our custom carrier board we've this regulator
> > always on and we use this GPIO for heartbeat LED, so I've this in our
> > carrier board DTS[2]:
> 
> Yes, you are absolutely right and e.g. on Apalis T30 [1] that is exactly how
> we did it.

And what about usdhc? It leads to the same overrides in the carrier board
file:

&iomuxc {
	usdhc {
		/* GPIO4_IO20 is LVDS */
		pinctrl_mmc_cd: gpio_mmc_cd {
		};

		/* Used as misc GPIOs */
		pinctrl_usdhc1: usdhc1grp {
		};
	};
};

> > > +/* PAD Ctrl values for common settings */
> > > +/*
> > > + * (PAD_CTL_HYS | PAD_CTL_PUS_100K_UP | PAD_CTL_PUE | PAD_CTL_PKE
> > > |
> > > + *  PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm)
> > > + */
> > > +#define PAD_CTRL_HYS_PU 0x1b0b0
> > 
> > This was requested to be reworked. I've simply replaced all the
> > macros with
> > hex values.
> 
> I don't think the reviewers really concluded on the action to be taken
> on here and just using hex values is probably the most stupid solution.

It's simple, if you just leave it as it is, then it will never get merged.
That's how kernel development works :) We've at least two options here:

1. Follow the old path and just convert those macros to hex values
2. Rework it - move those macros to some header file etc.

-- ynezz