On Thu, Jan 09, 2025 at 10:13:48AM +0530, Mukesh Kumar Savaliya wrote:
> Hi Kartik,
>
> On 1/8/2025 4:36 PM, Kartik Rajput wrote:
> > From: Akhil R <akhilrajeev@xxxxxxxxxx>
> >
> > Add support for Tegra264 SoC which supports 17 generic I2C controllers,
> > two of which are in the AON (always-on) partition of the SoC. Tegra264
> > I2C supports all the features supported by Tegra194 I2C controllers.
> >
> > Signed-off-by: Akhil R <akhilrajeev@xxxxxxxxxx>
> > Signed-off-by: Kartik Rajput <kkartik@xxxxxxxxxx>
> > ---
> > drivers/i2c/busses/i2c-tegra.c | 28 ++++++++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> > index 7b97c6d347ee..cf05937cb826 100644
> > --- a/drivers/i2c/busses/i2c-tegra.c
> > +++ b/drivers/i2c/busses/i2c-tegra.c
> > @@ -1646,7 +1646,35 @@ static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
> > .has_hs_mode_support = true,
> > };
> > +static const struct tegra_i2c_hw_feature tegra264_i2c_hw = {
> I could see 7 controllers have been already added. And this may keep
> growing.
I'm not sure I understand the concern here. This is IP that's been in
use ever since the first Tegra chip was released about 15 years ago.
It's quite normal that the list of supported hardware will grow over
time. At the same time there will be occasional improvements of the
hardware that require certain parameterization.
> Can we make either default set which is common for most of and change only
> sepcific fields ?
It's difficult to do. These are const structures on purpose so that they
can go into .rodata, so as such there's no good way to reuse defaults. I
suppose we could do something like add preprocessor defines, but I doubt
that they would make things any better (these are quite fine-grained, so
macros would likely only cover one or two fields at a time).
> Second option - read these fields from DT and overwrite default if it's
> mentioned in DTSI.
Some information is already parsed from DT. What's in this structure can
all be derived from the compatible string, hence why it's associated
with the compatible string via the of_device_id table. Moreover, we
cannot move any of this information out into device tree (at least not
for existing chips) because it would break DT ABI.
> Please review and see if this makes sense. what others say ?
I'm always open to suggestions, but I also don't see this as very
problematic. It's data that is cleanly structured out, not difficult to
maintain and doesn't take up a huge amount of space.
Thierry
Attachment:
signature.asc
Description: PGP signature
