Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 30 May 2021 09:58:01 +0100
From:   Phillip Potter <phil@philpotter.co.uk>
To:     Mark Brown <broonie@kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 55/69] ASoC: rt5645: add error checking to rt5645_probe
 function
Message-ID: <YLNTmRvIoJWv6lgG@equinox>
References: <20210503115736.2104747-1-gregkh@linuxfoundation.org>
 <20210503115736.2104747-56-gregkh@linuxfoundation.org>
 <YK1uZdtgffiCnUVQ@sirena.org.uk>
 <YK1z6AlOAMxYCh75@equinox>
 <YK/JfeMt9j9z+40Q@sirena.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YK/JfeMt9j9z+40Q@sirena.org.uk>
Precedence: bulk

On Thu, May 27, 2021 at 05:31:57PM +0100, Mark Brown wrote:
> On Tue, May 25, 2021 at 11:02:16PM +0100, Phillip Potter wrote:
> > On Tue, May 25, 2021 at 10:38:45PM +0100, Mark Brown wrote:
> 
> > > Phillip, please follow the standard patch submission process,
> > > this is documented in submitting-paches.rst in the kernel tree.
> > > In particular please make sure that you copy the relevant
> 
> > This patch was submitted to a closed mentoring group as part of the
> > University of Minnesota reversion/checking process. I was not
> > responsible for the final send out to the public mailing lists etc. as
> > the patches were collated first and sent out together en masse.
> 
> OK, this is really unfortunate.
> 
> > > This comment is not accurate, rt5645_remove() just resets the
> > > hardware - it's not going to clean up anything to do with any of
> > > the branches to error you've got above.  The core *will* clean up
> 
> > My comment was adjusted after submission for brevity's sake. This was
> > what I originally wrote:
> >  /*
> >   * All of the above is cleaned up when we return an error here, as
> >   * the caller will notice the error and invoke rt5645_remove and
> >   * other cleanup routines, as it does for the snd_soc_dapm_* calls
> >   * above as well.
> >   */
> > Happy to resubmit/rewrite as needed? Based on what you've written
> > though it may be better to drop the patch?
> 
> That is a lot better yes, it accurately reflects what was going
> on - the review definitely wasn't helping here.
> 
> > > Also I'm guessing this was done purely through inspection rather
> > > than the code having been tested?  If there was a problem seen at
> > > runtime this isn't fixing it, TBH I'm more than a little dubious
> 
> > Yes, that's correct - I did not test this directly other than making
> > sure it builds, as I don't have this hardware to test with. 
> 
> OK, in that case it's going to be safer to just drop the change,
> it's probably not going to cause any actual problems but it's
> certainly not something that should go in as a hurried fix.

Dear Mark,

Thank you, I will not resubmit with the new comment in that case. Have a
great weekend.

Regards,
Phil