Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 3 Feb 2020 10:40:35 +0100
From:   Daniel Vetter <daniel@ffwll.ch>
To:     Benjamin GAIGNARD <benjamin.gaignard@st.com>
Cc:     Dave Airlie <airlied@gmail.com>, Lyude Paul <lyude@redhat.com>,
        David Airlie <airlied@linux.ie>, Sean Paul <sean@poorly.run>,
        ML dri-devel <dri-devel@lists.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
Message-ID: <20200203094035.GR43062@phenom.ffwll.local>
Mail-Followup-To: Benjamin GAIGNARD <benjamin.gaignard@st.com>,
        Dave Airlie <airlied@gmail.com>, Lyude Paul <lyude@redhat.com>,
        David Airlie <airlied@linux.ie>, Sean Paul <sean@poorly.run>,
        ML dri-devel <dri-devel@lists.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
References: <20191128135057.20020-1-benjamin.gaignard@st.com>
 <878snsvxzu.fsf@intel.com>
 <CA+M3ks5WvYoDLSrbvaGBbJg9+nnkX=xyCiD389QD8tSCdNqB+g@mail.gmail.com>
 <CA+M3ks4Y4LemFh=dQds91Z-LGJPK3vHKv=GeUNYHjNhdwz_m2g@mail.gmail.com>
 <CA+M3ks4yEBejzMoXPw_OK_LNP7ag5SNXZjvHqNeuZ8+9r2X-qw@mail.gmail.com>
 <b273036b10d8c2882800d01dcda7392e93b731fa.camel@redhat.com>
 <CA+M3ks5cuC5yJ-e0DCUiY1HtyyeU=mM9z56y4e_UduKaxcbw-A@mail.gmail.com>
 <08f4b69b1e48a81e90f28e7672da15cc5165969c.camel@redhat.com>
 <CAPM=9txafoQNfMFf0Ff1SnBTgq6jYyvjJyjCSJua6-SJVVkScQ@mail.gmail.com>
 <f64197e6-74bd-6577-2aa7-9c69cfdb9080@st.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <f64197e6-74bd-6577-2aa7-9c69cfdb9080@st.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jan 31, 2020 at 08:08:34AM +0000, Benjamin GAIGNARD wrote:
> 
> On 1/31/20 12:22 AM, Dave Airlie wrote:
> >>>> hi-actually yes, we should probably be using this instead of just dropping
> >>>> this. Also, I didn't write this code originally I just refactored a bunch
> >>>> of
> >>>> it - Dave Airlied is the original author, but the original version of this
> >>>> code was written ages ago. tbh, I think it's a safe bet to say that they
> >>>> probably did mean to use this but forgot to and no one noticed until now.
> >>> Hi,
> >>>
> >>> Any clue about how to use crc value ? Does it have to be checked
> >>> against something else ?
> >>> If crc are not matching what should we do of the data copied just before ?
> >> We should be able to just take the CRC value from the sideband message and
> >> then generate our own CRC value using the sideband message contents, and check
> >> if the two are equal. If they aren't, something went wrong and we didn't
> >> receive the message properly.
> >>
> >> Now as to what we should do when we have CRC mismatches? That's a bit more
> >> difficult. If you have access to the DP MST spec, I suppose a place to start
> >> figuring that out would be checking if there's a way for us to request that a
> >> branch device resend whatever message it sent previously. If there isn't, I
> >> guess we should just print an error in dmesg (possibly with a hexdump of the
> >> failed message as well) and not forward the message to the driver. Not sure of
> >> any better way of handling it then that
> > Yeah I think this reflects what I wanted to do, I've no memory of a
> > retransmit option in the spec, but I've away from it for a while. But
> > we'd want to compare the CRC with what we got to make sure the are the
> > same.
> 
> Hmm, that far more complex than just fix compilation warnings :)
> 
> I will split the patch in two:
> 
> - one for of all other warnings, hopefully it can get reviewed
> 
> - one for this crc4 variable. Does checking crc value and print an error 
> should be acceptable ?
> 
> Something like:
> 
> if (crc4 != msg->chunk[msg->curchunk_len - 1])
> 
>  ??? print_hex_dump(KERN_DEBUG, "wrong crc", DUMP_PREFIX_NONE, 16, 1, 
> msg->chunk,? msg->curchunk_len, false);

Yeah I think that should be reasonable as a start. Then we'll see how much
the bug reports start flowing in ...
-Daniel
> 
> 
> Benjamin
> 
> 
> >
> > Dave.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch