[03/12] pata_parport: remove devtype from struct pi_adapter

Message ID 20230211144232.15138-4-linux@zary.sk
State New
Headers
Series pata_parport: protocol drivers cleanups |

Commit Message

Ondrej Zary Feb. 11, 2023, 2:42 p.m. UTC
  Only bpck driver uses devtype but it never gets set in pata_parport.
Remove it.

Signed-off-by: Ondrej Zary <linux@zary.sk>
---
 drivers/ata/pata_parport/bpck.c | 2 +-
 include/linux/pata_parport.h    | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)
  

Comments

Sergey Shtylyov Feb. 11, 2023, 7:11 p.m. UTC | #1
On 2/11/23 5:42 PM, Ondrej Zary wrote:

> Only bpck driver uses devtype but it never gets set in pata_parport.
> Remove it.
> 
> Signed-off-by: Ondrej Zary <linux@zary.sk>
> ---
>  drivers/ata/pata_parport/bpck.c | 2 +-
>  include/linux/pata_parport.h    | 3 ---
>  2 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/ata/pata_parport/bpck.c b/drivers/ata/pata_parport/bpck.c
> index b9174cf8863c..451a068fe28a 100644
> --- a/drivers/ata/pata_parport/bpck.c
> +++ b/drivers/ata/pata_parport/bpck.c
> @@ -241,7 +241,7 @@ static void bpck_connect ( PIA *pi  )
>  
>  	WR(5,8);
>  
> -	if (pi->devtype == PI_PCD) {
> +	if (1 /*pi->devtype == PI_PCD*/) {	/* FIXME */
>  		WR(0x46,0x10);		/* fiddle with ESS logic ??? */

   Why not drop this entire *if* stmt? 

>  		WR(0x4c,0x38);
>  		WR(0x4d,0x88);
[...]

MBR, Sergey
  
Ondrej Zary Feb. 11, 2023, 8:47 p.m. UTC | #2
On Saturday 11 February 2023 20:11:06 Sergey Shtylyov wrote:
> On 2/11/23 5:42 PM, Ondrej Zary wrote:
> 
> > Only bpck driver uses devtype but it never gets set in pata_parport.
> > Remove it.
> > 
> > Signed-off-by: Ondrej Zary <linux@zary.sk>
> > ---
> >  drivers/ata/pata_parport/bpck.c | 2 +-
> >  include/linux/pata_parport.h    | 3 ---
> >  2 files changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/ata/pata_parport/bpck.c b/drivers/ata/pata_parport/bpck.c
> > index b9174cf8863c..451a068fe28a 100644
> > --- a/drivers/ata/pata_parport/bpck.c
> > +++ b/drivers/ata/pata_parport/bpck.c
> > @@ -241,7 +241,7 @@ static void bpck_connect ( PIA *pi  )
> >  
> >  	WR(5,8);
> >  
> > -	if (pi->devtype == PI_PCD) {
> > +	if (1 /*pi->devtype == PI_PCD*/) {	/* FIXME */
> >  		WR(0x46,0x10);		/* fiddle with ESS logic ??? */
> 
>    Why not drop this entire *if* stmt? 

I decided to keep it (for now) as a marker of a possible bug. I currently don't have HW to test this driver.

> 
> >  		WR(0x4c,0x38);
> >  		WR(0x4d,0x88);
> [...]
> 
> MBR, Sergey
>
  
Damien Le Moal Feb. 12, 2023, 12:17 p.m. UTC | #3
On 2/12/23 05:47, Ondrej Zary wrote:
> On Saturday 11 February 2023 20:11:06 Sergey Shtylyov wrote:
>> On 2/11/23 5:42 PM, Ondrej Zary wrote:
>>
>>> Only bpck driver uses devtype but it never gets set in pata_parport.
>>> Remove it.
>>>
>>> Signed-off-by: Ondrej Zary <linux@zary.sk>
>>> ---
>>>  drivers/ata/pata_parport/bpck.c | 2 +-
>>>  include/linux/pata_parport.h    | 3 ---
>>>  2 files changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/ata/pata_parport/bpck.c b/drivers/ata/pata_parport/bpck.c
>>> index b9174cf8863c..451a068fe28a 100644
>>> --- a/drivers/ata/pata_parport/bpck.c
>>> +++ b/drivers/ata/pata_parport/bpck.c
>>> @@ -241,7 +241,7 @@ static void bpck_connect ( PIA *pi  )
>>>  
>>>  	WR(5,8);
>>>  
>>> -	if (pi->devtype == PI_PCD) {
>>> +	if (1 /*pi->devtype == PI_PCD*/) {	/* FIXME */
>>>  		WR(0x46,0x10);		/* fiddle with ESS logic ??? */
>>
>>    Why not drop this entire *if* stmt? 
> 
> I decided to keep it (for now) as a marker of a possible bug. I currently don't have HW to test this driver.

Then leave that if as-is and only add a comment detailing what needs to be
done (rather than just "FIXME"). This "if (1)" is just too odd and will
likely trigger code checker warnings.

> 
>>
>>>  		WR(0x4c,0x38);
>>>  		WR(0x4d,0x88);
>> [...]
>>
>> MBR, Sergey
>>
> 
>
  

Patch

diff --git a/drivers/ata/pata_parport/bpck.c b/drivers/ata/pata_parport/bpck.c
index b9174cf8863c..451a068fe28a 100644
--- a/drivers/ata/pata_parport/bpck.c
+++ b/drivers/ata/pata_parport/bpck.c
@@ -241,7 +241,7 @@  static void bpck_connect ( PIA *pi  )
 
 	WR(5,8);
 
-	if (pi->devtype == PI_PCD) {
+	if (1 /*pi->devtype == PI_PCD*/) {	/* FIXME */
 		WR(0x46,0x10);		/* fiddle with ESS logic ??? */
 		WR(0x4c,0x38);
 		WR(0x4d,0x88);
diff --git a/include/linux/pata_parport.h b/include/linux/pata_parport.h
index 9614ce53470a..3fc6b002c7c8 100644
--- a/include/linux/pata_parport.h
+++ b/include/linux/pata_parport.h
@@ -11,15 +11,12 @@ 
 
 #include <linux/libata.h>
 
-#define PI_PCD	1	/* dummy for paride protocol modules */
-
 struct pi_adapter {
 	struct device dev;
 	struct pi_protocol *proto;	/* adapter protocol */
 	int port;			/* base address of parallel port */
 	int mode;			/* transfer mode in use */
 	int delay;			/* adapter delay setting */
-	int devtype;			/* dummy for paride protocol modules */
 	char *device;			/* dummy for paride protocol modules */
 	int unit;			/* unit number for chained adapters */
 	int saved_r0;			/* saved port state */