Re: [PATCH v2] pata_parport: add driver (PARIDE replacement)

[Ondrej Zary]

On Monday 23 January 2023 02:03:16 Damien Le Moal wrote:

[...]

> You are missing:
> 
> #ifndef LINUX_PATA_PARPORT_H
> #define LINUX_PATA_PARPORT_H
> 
> > +#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 */
> > +	int saved_r2;			/* saved port state */
> > +	unsigned long private;		/* for protocol module */
> > +	struct pardevice *pardev;	/* pointer to pardevice */
> > +};
> > +
> > +typedef struct pi_adapter PIA;	/* for paride protocol modules */
> > +
> > +/* registers are addressed as (cont,regr)
> > + *	cont: 0 for command register file, 1 for control register(s)
> > + *	regr: 0-7 for register number.
> > + */
> > +
> > +/* macros and functions exported to the protocol modules */
> > +#define delay_p			(pi->delay ? udelay(pi->delay) : (void)0)
> > +#define out_p(offs, byte)	do { outb(byte, pi->port + offs); delay_p; } while (0)
> > +#define in_p(offs)		(delay_p, inb(pi->port + offs))
> 
> It would be way nicer to have these as inline functions.

This should go away (as well as the code below) so I'm not touching it now.

> > +
> > +#define w0(byte)		out_p(0, byte)
> > +#define r0()			in_p(0)
> > +#define w1(byte)		out_p(1, byte)
> > +#define r1()			in_p(1)
> > +#define w2(byte)		out_p(2, byte)
> > +#define r2()			in_p(2)
> > +#define w3(byte)		out_p(3, byte)
> > +#define w4(byte)		out_p(4, byte)
> > +#define r4()			in_p(4)
> > +#define w4w(data)		do { outw(data, pi->port + 4); delay_p; } while (0)
> > +#define w4l(data)		do { outl(data, pi->port + 4); delay_p; } while (0)
> > +#define r4w()			(delay_p, inw(pi->port + 4))
> > +#define r4l()			(delay_p, inl(pi->port + 4))
> > +
> > +static inline u16 pi_swab16(char *b, int k)
> > +{
> > +	union { u16 u; char t[2]; } r;
> > +
> > +	r.t[0] = b[2 * k + 1]; r.t[1] = b[2 * k];
> > +	return r.u;
> > +}
> > +
> > +static inline u32 pi_swab32(char *b, int k)
> > +{
> > +	union { u32 u; char f[4]; } r;
> > +
> > +	r.f[0] = b[4 * k + 1]; r.f[1] = b[4 * k];
> > +	r.f[2] = b[4 * k + 3]; r.f[3] = b[4 * k + 2];
> > +	return r.u;
> > +}

-- 
Ondrej Zary