Received-SPF: pass (google.com: domain of linux-kernel+bounces-110091-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Feedback-ID: ie8e14432:Fastmail
Date: Thu, 21 Mar 2024 22:25:18 +0900
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: Adam Goldman <adamg@pobox.com>
Cc: linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] firewire: core: use long bus reset on gap count error
Message-ID: <20240321132518.GA755964@workstation.local>
Mail-Followup-To: Adam Goldman <adamg@pobox.com>,
	linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
References: <ZdxH-mjmd09jmYZN@iguana.24-8.net>
 <20240228004144.GA72391@workstation.local>
 <ZfqufddlPhxzcDCN@iguana.24-8.net>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZfqufddlPhxzcDCN@iguana.24-8.net>

Hi Adam,

Thanks for the patches to improve the subsystem.

Inconveniently to you , we are now just at the merge window for v6.9
kernel, thus I would not put any changes except for the changes to
Linus. I'd like you to wait until the next week, sorry.

However, in the topic of logging PHY register, I have an idea to utilize
the Linux kernel tracepoints framework[1]. It is tangled to program with
the provided macros, and it is available just with the relevant tools[2],
but it would be helpful in the case, I think.

[1] https://docs.kernel.org/trace/tracepoints.html
[2] https://docs.kernel.org/trace/tracepoint-analysis.html

On Wed, Mar 20, 2024 at 02:38:05AM -0700, Adam Goldman wrote:
> Hi Takashi,
> 
> On Wed, Feb 28, 2024 at 09:41:44AM +0900, Takashi Sakamoto wrote:
> > Additionally, for your investigation, you added the debug print to get the
> > timing of bus reset scheduling. I think it useful for this kind of issue.
> > Would I ask you to write another patch to add it? In my opinion, the case
> > of mixed versions of 1394 PHYs in the same bus has more quirks and the
> > debug print is helpful to investigate it further.
> 
> I'm sorry for my delay in preparing a patch.
> 
> I've submitted a patch to linux1394-devel to log when we schedule or 
> initiate a bus reset. This is enabled with a new parameter to the 
> firewire-core module. It provides logging similar to the debug print I 
> used to investigate the reset loop.
> 
> Also, there is already logging for bus reset interrupts in 
> firewire-ohci. This logs all bus resets and does not indicate whether we 
> initiated the reset or some other node on the bus initiated it. However, 
> the logging in firewire-ohci always froze my computer when I enabled it. 
> I've submitted a separate patch to fix the firewire-ohci logging.
> 
> I believe both forms of logging can be useful. firewire-ohci logs all 
> bus resets, but it doesn't tell where the resets came from. firewire-core 
> only logs bus resets we initiate.
> 
> I also considered adding an option to firewire-ohci to log PHY register 
> access. This would include writes to IBR and ISBR, so it would log when 
> we initiate resets. However, this logging would be more complicated to 
> add, so I didn't do it.
> 
> -- Adam


Thanks

Takashi Sakamoto