Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

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

 




On Thursday 05 May 2016 07:03 PM, Jon Hunter wrote:
On 05/05/16 14:09, Laxman Dewangan wrote:
On Thursday 05 May 2016 06:38 PM, Jon Hunter wrote:
On 05/05/16 11:32, Laxman Dewangan wrote:
On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
On 04/05/16 12:39, Laxman Dewangan wrote:
+        return -EINVAL;
+
+    for (i = 0; i < soc->num_io_pads; ++i) {
+        if (soc->io_pads_control[i].pad_id == pad_id)
+            return soc->io_pads_control[i].dpd_bit_pos;
+    }
Do we need a loop here? Can't we just make the table a look-up table
now
that the ID is just an index?
We do not support the table for all pads and so for those non supported
pad index, it will be 0 (default) and 0 is the valid bit position here.
That does make it tricky.

If you want table then we will need one more information for making that
index as valid/invalid.
We can pack the valid/invalid with bit position to make u32.
Another option would be, to have a single table for all devices and the
make the valid field a valid mask which has a bit for each SoC.
We have 2 register for DPD and hence making the mask bit will need u64.

I think we can have like below to avoid loop.
struct tegra_io_pads_control {
         int dpd_supported;
         int voltage_change_supported;
         int dpd_config_bit;
         int voltage_config_bit;
};
Why can't we have ...

struct tegra_io_pads_control {
         int dpd_config_bit;
         int voltage_config_bit;
	unsigned int soc_mask;
};

Then .valid should indicate if it the IO pads group is valid for the
device ...

	.soc_mask = TEGRA_IO_PADS_T124
or
	.soc_mask = TEGRA_IO_PADS_T210
or
	.soc_mask = TEGRA_IO_PADS_T124 | TEGRA_IO_PADS_T210

You can use -1 to indicate the for the dpd and voltage bit to indicate
if they are valid. In other words, you need to check the IO pad is valid
for the soc and then the bit is valid.

This will also work.
Then this is not required part of the soc data.
Only soc_io_mask need to be part of soc data.
This table can be file static.



I think that this is exactly what enums are for, then you don't have to
explicitly define each number.

We have defines in the dt binding header.
Nothing to stop us including the dt binding header in the pmc.c. We do
this for tegra clks.
The dt binding header is not there and need to add.
Part of this patch or different patch?

Also we can not have enums in binding header. Only macros/defines.

We do not have the dt binding doc yet as it will be in future patch.


BTW, are you fine to keep TEGRA_IO_PAD_* as defines instead of enums.
This is what POWERGATE are there.
Up to you, I prefer an enum. The POWERGATE IDs defines match the bit in
the register so it makes sense these are explicit.

OK, let me make enums. Last member as MAX so that I can initialize the tegra_io_pad_info[TEGRA_IO_PAD_MAX] =

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



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux