linux-kernel - Re: [RESEND PATCH v4 0/8] i2c: Relax mandatory I2C ID table passing

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: <CAB3z_RpmZsxL6=UiYf7s2BM_OSKUkd0vOJ7GxVER-oh+=XhK+w@mail.gmail.com>
Date:	Thu, 1 Oct 2015 22:10:57 +0100
From:	Kieran Bingham <kieranbingham@...il.com>
To:	Wolfram Sang <wsa@...-dreams.de>
Cc:	Lee Jones <lee.jones@...aro.org>,
	Javier Martinez Canillas <javier@....samsung.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
	Grant Likely <grant.likely@...aro.org>
Subject: Re: [RESEND PATCH v4 0/8] i2c: Relax mandatory I2C ID table passing

Hi Wolfram,

On 1 October 2015 at 21:50, Wolfram Sang <wsa@...-dreams.de> wrote:
>
>> > Yes but that is not true for drivers that support both OF and legacy board
>> > files. For those drivers, there will be a lot of boiler plate code duplicated
>> > that would look something like:
>> >
>> >      unsigned long data;
>> >      struct of_device_id *match;
>> >      struct i2c_devicd_id *id;
>> >
>> >      if (i2c->dev.of_node) {
>> >             match = i2c_of_match_device(of_match_table, i2c);
>> >         if (!match)
>> >                return -EINVAL;
>> >
>> >             data = (unsigned long)match->data;
>> >      } else {
>> >             id = i2c_match_id(id_table, i2c);
>> >         if (!id)
>> >                return -EINVAL;
>> >
>> >             data = id->driver_data;
>> >      }
>
> I said this before: It is not only the additional code, I think it is
> quite unelegant to to do the matching again which has already been done.
> (and DT boottime has already increased, partly due to the excessive
> string matching). Also, I wouldn't like to see an I2C specific solution;
> this problem exists for other subsystems, too.
>
>> I'm fine with a new API for this stuff.  I'm even happy to go ahead
>> and code it up, but it's important to note that this is work which
>> should be based on this set and not a blocker for this set to be
>> accepted.
>
> Is that a promise? :)

Well if Lee doesn't, then I'll be trying to take it on.
I've already written an spatch to help with the conversion of other
drivers to follow on for this series.

Between us I think we've got motivation to make progress.

>> The correct approach is the former.  One of the aims of this set was
>> to bring the I2C .probe() call-back more into line with the majority
>> of the other .probe() calls in the kernel i.e. with only a single
>> parameter.  I'm really not a fan of passing some random void pointer
>
> Yes, I like this about this series.
>
>> in.  Using a look-up call to fetch ACPI/OF/I2C/etc data is the current
>> norm and is a very viable option.
>
> It is the status quo, but that doesn't make it better IMO.
>
>> Wolfram, please (finally :D) take this set.
>
> I tend to give in ;) Maybe we can talk in Dublin a bit about a possible
> next step after this series?

I don't think Lee is coming to Dublin, but I'll be there all week, if
you find time for a chat.
I'll look out for you in the hallway track, or at least on stage for
the final ELCE games :)

>
> Thanks,
>
>    Wolfram
>

--
Regards

Kieran
--
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/