Re: [PATCH 4/4] pinctrl: tegra: add pinmux controller driver for Tegra124

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday 06 December 2013 05:00 AM, Stephen Warren wrote:
On 12/05/2013 03:57 AM, Laxman Dewangan wrote:
From: Ashwini Ghuge <aghuge@xxxxxxxxxx>

This adds a driver for the Tegra124 pinmux, and required
parameterization data for Tegra124.

The driver uses the common Tegra pincontrol driver utility
functions to implement the majority of the driver.

This driver is not compatible with the earlier NVIDIA's SoCs,
hence add new compatibile as "nvidia,tegra124-pinmux".

Originally written by Ashwini Gguhe.
ldewangan:
         - cleanup the patches,
         - Fix address issue.
IIRC, Thierry mentioned he had some fixes in his local branch for this
driver. Thierry, can you please confirm/deny this?

I made the following comment on the version Ashwini posted, which hasn't
been addressed yet:

A day or two ago during upstream review:

Yes, we have some missing entry and I responded into other mail thread. I will post the diff here to complete.


+static const struct tegra_function  tegra124_functions[] = {
...
+	FUNCTION(i2c1),
+	FUNCTION(i2c2),
+	FUNCTION(i2c3),
+	FUNCTION(i2c4),
+	FUNCTION(i2cpwr),
Is that complete? Tegra124 apparently has 6 I2C controllers. Are the
pins for the new sixth controller (0x7000d100) not affected by the pinmux?
That said, if we find things are missing, I suppose we can add them
later without breaking existing ABI. Breakage would only happen if we
had to change/remove something.

During downstream review quite a while ago I also said:

I2C6 pinmux is not in this controller, it is dpaux controller and we have not supported this. This require little bit different handling. I have already downstream bug for support this. Will add you in loop so that single fix can work for mainline and downstream.

Other than I2C6, some other mux are missing.


+static const struct pinctrl_pin_desc  tegra124_pins[] = {
There are two spaces before "tegra124_pins[]".

+static const char * const gmi_groups[] = {
+	"uart2_cts_n_pj5",
+	"uart2_rts_n_pj6",
+	"uart3_txd_pw6",
+	"uart3_rxd_pw7",
+	"uart3_cts_n_pa1",
+	"uart3_rts_n_pc0",
+
+	"pu0",
It'd be best not to have blank lines in the middle of arrays. The same comment exists elsewhere in the
file, so make sure you search the whole file.
Nits:

- There are some cases of multiple blank lines back-to-back.
- There's a blank line at the end of the file.

Aside from those minor issues, patches 1/4 and 4/4,
Acked-by: Stephen Warren <swarren@xxxxxxxxxx>

(BTW, those 2 patches would go through the pinctrl tree, and patches 2/4
and 3/4 would go through the Tegra tree. You generally shouldn't posted
patches that will be applied to different trees in the same series,
since there aren't dependencies).
Fine.
Should I sent the diff or full change? I think full change as this need to go on pincontrol subsystem, not in Tegra.

Thanks,
Laxman


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux