From: Heiko Stuebner <heiko.stuebner@cherry.de>

The variant of the driver in the vendor-tree contained those handy
comments in the regmap register callback. Having the different ranges
describe what they are looks helpful.

Signed-off-by: Heiko Stuebner <heiko.stuebner@cherry.de>
---
 drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
index 4ced1479e2dc..c5c64c209e96 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
@@ -766,13 +766,13 @@ static struct tx_drv_ctrl tx_drv_ctrl_hbr2[4][4] = {
 static bool rk_hdptx_phy_is_rw_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
-	case 0x0000 ... 0x029c:
-	case 0x0400 ... 0x04a4:
-	case 0x0800 ... 0x08a4:
-	case 0x0c00 ... 0x0cb4:
-	case 0x1000 ... 0x10b4:
-	case 0x1400 ... 0x14b4:
-	case 0x1800 ... 0x18b4:
+	case 0x0000 ... 0x029c: /* CMN Register */
+	case 0x0400 ... 0x04a4: /* Sideband Register */
+	case 0x0800 ... 0x08a4: /* Lane Top Register */
+	case 0x0c00 ... 0x0cb4: /* Lane 0 Register */
+	case 0x1000 ... 0x10b4: /* Lane 1 Register */
+	case 0x1400 ... 0x14b4: /* Lane 2 Register */
+	case 0x1800 ... 0x18b4: /* Lane 3 Register */
 		return true;
 	}
 
-- 
2.45.2

On 12/6/24 12:34 PM, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@cherry.de>
> 
> The variant of the driver in the vendor-tree contained those handy
> comments in the regmap register callback. Having the different ranges
> describe what they are looks helpful.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@cherry.de>
> ---
>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> index 4ced1479e2dc..c5c64c209e96 100644
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> @@ -766,13 +766,13 @@ static struct tx_drv_ctrl tx_drv_ctrl_hbr2[4][4] = {
>  static bool rk_hdptx_phy_is_rw_reg(struct device *dev, unsigned int reg)
>  {
>  	switch (reg) {
> -	case 0x0000 ... 0x029c:
> -	case 0x0400 ... 0x04a4:
> -	case 0x0800 ... 0x08a4:
> -	case 0x0c00 ... 0x0cb4:
> -	case 0x1000 ... 0x10b4:
> -	case 0x1400 ... 0x14b4:
> -	case 0x1800 ... 0x18b4:
> +	case 0x0000 ... 0x029c: /* CMN Register */
> +	case 0x0400 ... 0x04a4: /* Sideband Register */
> +	case 0x0800 ... 0x08a4: /* Lane Top Register */
> +	case 0x0c00 ... 0x0cb4: /* Lane 0 Register */
> +	case 0x1000 ... 0x10b4: /* Lane 1 Register */
> +	case 0x1400 ... 0x14b4: /* Lane 2 Register */
> +	case 0x1800 ... 0x18b4: /* Lane 3 Register */
>  		return true;
>  	}
>  

Nice finding!  The HDMI variant of the driver which I used as an
implementation reference (i.e. phy-rockchip-samsung-hdptx-hdmi.c)
doesn't provide these comments.

Reviewed-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

Am Freitag, 6. Dezember 2024, 12:18:56 CET schrieb Cristian Ciocaltea:
> On 12/6/24 12:34 PM, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@cherry.de>
> > 
> > The variant of the driver in the vendor-tree contained those handy
> > comments in the regmap register callback. Having the different ranges
> > describe what they are looks helpful.
> > 
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@cherry.de>
> > ---
> >  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> > index 4ced1479e2dc..c5c64c209e96 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> > @@ -766,13 +766,13 @@ static struct tx_drv_ctrl tx_drv_ctrl_hbr2[4][4] = {
> >  static bool rk_hdptx_phy_is_rw_reg(struct device *dev, unsigned int reg)
> >  {
> >  	switch (reg) {
> > -	case 0x0000 ... 0x029c:
> > -	case 0x0400 ... 0x04a4:
> > -	case 0x0800 ... 0x08a4:
> > -	case 0x0c00 ... 0x0cb4:
> > -	case 0x1000 ... 0x10b4:
> > -	case 0x1400 ... 0x14b4:
> > -	case 0x1800 ... 0x18b4:
> > +	case 0x0000 ... 0x029c: /* CMN Register */
> > +	case 0x0400 ... 0x04a4: /* Sideband Register */
> > +	case 0x0800 ... 0x08a4: /* Lane Top Register */
> > +	case 0x0c00 ... 0x0cb4: /* Lane 0 Register */
> > +	case 0x1000 ... 0x10b4: /* Lane 1 Register */
> > +	case 0x1400 ... 0x14b4: /* Lane 2 Register */
> > +	case 0x1800 ... 0x18b4: /* Lane 3 Register */
> >  		return true;
> >  	}
> >  
> 
> Nice finding!  The HDMI variant of the driver which I used as an
> implementation reference (i.e. phy-rockchip-samsung-hdptx-hdmi.c)
> doesn't provide these comments.

yep, those comments were in the displayport variant of the phy-driver ;-) .
I don't really understand why the vendor tree contains 2 separate drivers
written by two different people, but at least it looks like we're getting
one driver in mainline, thanks to Damon's work :-)

On 12/6/24 7:36 PM, Heiko Stübner wrote:
> Am Freitag, 6. Dezember 2024, 12:18:56 CET schrieb Cristian Ciocaltea:
>> On 12/6/24 12:34 PM, Heiko Stuebner wrote:
>>> From: Heiko Stuebner <heiko.stuebner@cherry.de>
>>>
>>> The variant of the driver in the vendor-tree contained those handy
>>> comments in the regmap register callback. Having the different ranges
>>> describe what they are looks helpful.
>>>
>>> Signed-off-by: Heiko Stuebner <heiko.stuebner@cherry.de>
>>> ---
>>>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 14 +++++++-------
>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>>> index 4ced1479e2dc..c5c64c209e96 100644
>>> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>>> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>>> @@ -766,13 +766,13 @@ static struct tx_drv_ctrl tx_drv_ctrl_hbr2[4][4] = {
>>>  static bool rk_hdptx_phy_is_rw_reg(struct device *dev, unsigned int reg)
>>>  {
>>>  	switch (reg) {
>>> -	case 0x0000 ... 0x029c:
>>> -	case 0x0400 ... 0x04a4:
>>> -	case 0x0800 ... 0x08a4:
>>> -	case 0x0c00 ... 0x0cb4:
>>> -	case 0x1000 ... 0x10b4:
>>> -	case 0x1400 ... 0x14b4:
>>> -	case 0x1800 ... 0x18b4:
>>> +	case 0x0000 ... 0x029c: /* CMN Register */
>>> +	case 0x0400 ... 0x04a4: /* Sideband Register */
>>> +	case 0x0800 ... 0x08a4: /* Lane Top Register */
>>> +	case 0x0c00 ... 0x0cb4: /* Lane 0 Register */
>>> +	case 0x1000 ... 0x10b4: /* Lane 1 Register */
>>> +	case 0x1400 ... 0x14b4: /* Lane 2 Register */
>>> +	case 0x1800 ... 0x18b4: /* Lane 3 Register */
>>>  		return true;
>>>  	}
>>>  
>>
>> Nice finding!  The HDMI variant of the driver which I used as an
>> implementation reference (i.e. phy-rockchip-samsung-hdptx-hdmi.c)
>> doesn't provide these comments.
> 
> yep, those comments were in the displayport variant of the phy-driver ;-) .
> I don't really understand why the vendor tree contains 2 separate drivers

Yeah, I asked myself the same question a while ago - I'd assume the
(initial) development happened in parallel, but the final sync/merge was
never done.

> written by two different people, but at least it looks like we're getting
> one driver in mainline, thanks to Damon's work :-)

Indeed, we're eventually getting there.. :-)