From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Thompson Subject: Re: [PATCH 2/3] backlight/arcxcnn fix vendor prefix Date: Fri, 21 Jun 2019 14:39:01 +0100 Message-ID: References: <1541592640-18478-1-git-send-email-bdodge09@gmail.com> <1541592640-18478-3-git-send-email-bdodge09@gmail.com> <20181111113053.GF27666@amd> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0641881323==" Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Brian Dodge Cc: Mark Rutland , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Jingoo Han , DRI Development , Rob Herring , Jacek Anaszewski , Pavel Machek , Peter Bacon , Lee Jones , Linux LED Subsystem List-Id: linux-leds@vger.kernel.org --===============0641881323== Content-Type: multipart/alternative; boundary="00000000000076ce46058bd596e2" --00000000000076ce46058bd596e2 Content-Type: text/plain; charset="UTF-8" Hi Brian On Tue, 27 Nov 2018 at 00:44, Brian Dodge wrote: > Thank you Pavel, that is a good point. > > The chip vendor has indicated that there is no reason to maintain the > old/improper prefix and wishes to go forward (only) with the "arctic" > prefix and any existing dts files are or will be updated. > Looks like this patch series has fallen into the cracks a little. I think I assumed this info would end in the description of patch v2 1/3 (in order to answer Rob's feedback) and I sat and waited for a respin. On the other hand... I didn't actually say that explicitly anywhere! So... I'd recommend a respin perhaps with a small bit of text explaining how the vendor can state that any existing dts files will be updated. This is a peripheral device so these strings are probably embedded into OEM devicetrees rather than exclusively under the control of the vendor. Daniel. > On 11/11/18 6:30 AM, Pavel Machek wrote: > > Hi! > > > >> The vendor-prefixes.txt file properly refers to ArcticSand > >> as arctic but the driver improperly abbreviated the prefix > >> to arc. This was a mistake in the original patch > >> > >> Signed-off-by: Brian Dodge > >> --- > >> drivers/video/backlight/arcxcnn_bl.c | 22 +++++++++++----------- > >> 1 file changed, 11 insertions(+), 11 deletions(-) > >> > >> * > >> - * Copyright 2016 ArcticSand, Inc. > >> - * Author : Brian Dodge > >> + * Copyright 2018 pSemi, Inc. > >> + * Author : Brian Dodge > > Ummm. Copyright 2016-2018? > > > >> @@ -202,27 +202,27 @@ static void arcxcnn_parse_dt(struct arcxcnn *lp) > >> if (ret == 0) > >> lp->pdata->initial_brightness = prog_val; > >> > >> - ret = of_property_read_u32(node, "arc,led-config-0", &prog_val); > >> + ret = of_property_read_u32(node, "arctic,led-config-0", &prog_val); > >> if (ret == 0) > >> lp->pdata->led_config_0 = (u8)prog_val; > >> > > If there's a dts using this, you want to update it at the same > > time. You may want to support both names going forward. > > > Pavel > --00000000000076ce46058bd596e2 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Brian

On Tue, 27 Nov 2018 at 00:44, Brian Dodge <= ;bdodge09@gmail.com> wrote:
Thank you Pavel, t= hat is a good point.

The chip vendor has indicated that there is no reason to maintain the
old/improper prefix and wishes to go forward (only) with the "arctic&q= uot;
prefix and any existing dts files are or will be updated.
<= div>
Looks like this patch series has fallen into the cracks = a little.

I think I assumed this info would end in= the description of patch v2 1/3 (in order to answer Rob's feedback) an= d I sat and waited for a respin. On the other hand... I didn't actually= say that explicitly anywhere! So... I'd recommend a respin perhaps wit= h a small bit of text explaining how the vendor can state that any existing= dts files will be updated. This is a peripheral device so these strings ar= e probably embedded into OEM devicetrees rather than exclusively under the = control of the vendor.


Daniel.




On 11/11/18 6:30 AM, Pavel Machek wrote:
> Hi!
>
>> The vendor-prefixes.txt file properly refers to ArcticSand
>> as arctic but the driver improperly abbreviated the prefix
>> to arc. This was a mistake in the original patch
>>
>> Signed-off-by: Brian Dodge <bdodge09@gmail.com>
>> ---
>>=C2=A0 =C2=A0drivers/video/backlight/arcxcnn_bl.c | 22 +++++++++++-= ----------
>>=C2=A0 =C2=A01 file changed, 11 insertions(+), 11 deletions(-)
>>
>>=C2=A0 =C2=A0 *
>> - * Copyright 2016 ArcticSand, Inc.
>> - * Author : Brian Dodge <bdodge@arcticsand.com>
>> + * Copyright 2018 pSemi, Inc.
>> + * Author : Brian Dodge <bdodge@psemi.com>
> Ummm. Copyright 2016-2018?
>
>> @@ -202,27 +202,27 @@ static void arcxcnn_parse_dt(struct arcxcnn = *lp)
>>=C2=A0 =C2=A0 =C2=A0 if (ret =3D=3D 0)
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 lp->pdata->i= nitial_brightness =3D prog_val;
>>
>> -=C2=A0 =C2=A0 ret =3D of_property_read_u32(node, "arc,led-co= nfig-0", &prog_val);
>> +=C2=A0 =C2=A0 ret =3D of_property_read_u32(node, "arctic,led= -config-0", &prog_val);
>>=C2=A0 =C2=A0 =C2=A0 if (ret =3D=3D 0)
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 lp->pdata->l= ed_config_0 =3D (u8)prog_val;
>>
> If there's a dts using this, you want to update it at the same
> time. You may want to support both names going forward.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Pavel
--00000000000076ce46058bd596e2-- --===============0641881323== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs --===============0641881323==--