[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMxnaaWup4=wkpm0W8rEsAq_mdFSiG8dn4uqYtfn6DcN9ztyhw@mail.gmail.com>
Date: Sun, 21 May 2017 15:46:20 +0200
From: Andreas Noever <andreas.noever@...il.com>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Michael Jamet <michael.jamet@...el.com>,
Yehezkel Bernat <yehezkel.bernat@...el.com>,
Lukas Wunner <lukas@...ner.de>,
Amir Levy <amir.jer.levy@...el.com>,
Andy Lutomirski <luto@...nel.org>, Mario.Limonciello@...l.com,
Jared.Dominguez@...l.com,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 02/24] thunderbolt: Do not try to read UID if DROM offset
is read as 0
On Thu, May 18, 2017 at 4:38 PM, Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
> At least Falcon Ridge when in host mode does not have any kind of DROM
> available and reading DROM offset returns 0 for these. Do not try to
> read DROM any further in that case.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> Reviewed-by: Yehezkel Bernat <yehezkel.bernat@...el.com>
> Reviewed-by: Michael Jamet <michael.jamet@...el.com>
> ---
> drivers/thunderbolt/eeprom.c | 3 +++
> 1 file changed, 3 insertions(+)
>
Hi Mika,
nice work, it is nice to see Intel contribute to the Thunderbolt
driver (I can second Lukas's 'jaw drop' comment)!
I will try to read through everything today, but maybe the last few
patches will get pushed back to next weekend.
> diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
> index 6392990c984d..e4e64b130514 100644
> --- a/drivers/thunderbolt/eeprom.c
> +++ b/drivers/thunderbolt/eeprom.c
> @@ -276,6 +276,9 @@ int tb_drom_read_uid_only(struct tb_switch *sw, u64 *uid)
> if (res)
> return res;
>
> + if (drom_offset == 0)
> + return -ENODEV;
> +
I think that this will make tb_switch_resume bail out on the root
switch, which is not good. Since the uid is only used to detect
whether a different device was plugged in while the system was
suspended I think that we can safely ignore the uid on the root
switch:
- don't read it in tb_drom_read (route == 0 is already special cased anyways)
- add a special case for the root switch to tb_switch_resume and
don't read the uid - just assume that it did not change (should be
impossible anyways)
What do you think?
Andreas
> /* read uid */
> res = tb_eeprom_read_n(sw, drom_offset, data, 9);
> if (res)
> --
> 2.11.0
>
Powered by blists - more mailing lists