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
| ||
|
Date: Wed, 6 Dec 2017 20:14:02 +0530 From: Vinod Koul <vinod.koul@...el.com> To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com> Cc: ALSA <alsa-devel@...a-project.org>, Charles Keepax <ckeepax@...nsource.cirrus.com>, Takashi <tiwai@...e.de>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, plai@...eaurora.org, LKML <linux-kernel@...r.kernel.org>, Sagar Dharia <sdharia@...eaurora.org>, patches.audio@...el.com, Mark <broonie@...nel.org>, srinivas.kandagatla@...aro.org, Sudheer Papothi <spapothi@...eaurora.org>, alan@...ux.intel.com Subject: Re: [alsa-devel] [PATCH v4 06/15] soundwire: Add IO transfer On Wed, Dec 06, 2017 at 07:32:43AM -0600, Pierre-Louis Bossart wrote: > On 12/5/17 11:58 PM, Vinod Koul wrote: > >On Tue, Dec 05, 2017 at 08:48:03AM -0600, Pierre-Louis Bossart wrote: > >>On 12/5/17 7:43 AM, Pierre-Louis Bossart wrote: > >>>On 12/5/17 12:31 AM, Vinod Koul wrote: > >>>>On Sun, Dec 03, 2017 at 09:01:41PM -0600, Pierre-Louis Bossart wrote: > > > >>>>>>>>+static inline int do_transfer(struct sdw_bus *bus, struct > >>>>>>>>sdw_msg *msg) > >>>>>>>>+{ > >>>>>>>>+ int retry = bus->prop.err_threshold; > >>>>>>>>+ enum sdw_command_response resp; > >>>>>>>>+ int ret = 0, i; > >>>>>>>>+ > >>>>>>>>+ for (i = 0; i <= retry; i++) { > >>>>>>>>+ resp = bus->ops->xfer_msg(bus, msg); > >>>>>>>>+ ret = find_response_code(resp); > >>>>>>>>+ > >>>>>>>>+ /* if cmd is ok or ignored return */ > >>>>>>>>+ if (ret == 0 || ret == -ENODATA) > >>>>>>> > >>>>>>>Can you document why you don't retry on a CMD_IGNORED? I know > >>>>>>>there was a > >>>>>>>reason, I just can't remember it. > >>>>>> > >>>>>>CMD_IGNORED can be okay on broadcast. User of this API can retry all > >>>>>>they > >>>>>>want! > >>>>> > >>>>>So you retry if this is a CMD_FAILED but let higher levels retry for > >>>>>CMD_IGNORED, sorry I don't see the logic. > >>>> > >>>>Yes that is right. > >>>> > >>>>If I am doing a broadcast read, lets say for Device Id registers, why in > >>>>the > >>>>world would I want to retry? CMD_IGNORED is a valid response and > >>>>required to > >>>>stop enumeration cycle in that case. > > > >Above is the clarfication > > > >>>>But if I am not expecting a CMD_IGNORED response, I can very well go > >>>>ahead > >>>>and retry from caller. The context is with caller and they can choose to > >>>>do > >>>>appropriate handling. > >>>> > >>>>And I have clarified this couple of times to you already, not sure how > >>>>many > >>>>more times I would have to do that. > >>> > >>>Until you clarify what you are doing. > > > >Let me try again, I think u missed that part of my reply above > > > >If I am doing a broadcast read, lets say for Device Id registers, > >why in the world would I want to retry? CMD_IGNORED is a valid response > >and required to stop enumeration cycle in that case. > > > >>>There is ONE case where IGNORED is a valid answer (reading the Prepare not > >>>finished bits), and it should not only be documented but analyzed in more > >>>details. > >>I meant Read SCP_DevID registers from Device0... prepare bits should never > >>return a CMD_IGNORED > > > >Precisely as I pointed out above. > > > >>>For a write an IGNORED is never OK. > > > >Agreed, but then transfer does both read and write. Write call can treat it > >as error and read call leaves it upto caller. > > > >Does that sound okay for you? > > Not really. > You have one case where it's ok and all others are not ok, to me this sounds > like you should avoid the retry at the lowest level rather than pushing this > to the caller > And now that I think of it, the definitions for the DisCo spec were really > meant for hardware-based retries available in some master IPs. If this is > enabled, then the loops in software are not really needed at all. Can you > please check this point? Sure I will check that -- ~Vinod
Powered by blists - more mailing lists