netdev - Re: [PATCH RFC net-next v1 1/9] ptp: introduce programmable pins.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1394455989.2296.23.camel@e37108.spectralink.com>
Date:	Mon, 10 Mar 2014 12:53:09 +0000
From:	Sørensen, Stefan 
	<Stefan.Sorensen@...ctralink.com>
To:	"richardcochran@...il.com" <richardcochran@...il.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"bhutchings@...arflare.com" <bhutchings@...arflare.com>,
	"christian.riesch@...cron.at" <christian.riesch@...cron.at>,
	"davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH RFC net-next v1 1/9] ptp: introduce programmable pins.

On Sat, 2014-03-08 at 20:42 +0100, Richard Cochran wrote:

>+int ptp_setpin(struct ptp_clock *ptp, unsigned int pin,
>+               enum ptp_pin_function func, unsigned int chan)

From the name I would think that this would set the state of the pin.
Rename to ptp_set_pinfunc?

> +int ptp_find_pin(struct ptp_clock *ptp,
> +		 enum ptp_pin_function func, unsigned int chan)
> +{
> +	struct ptp_pin_desc *pin = NULL;
> +	int i;
> +
> +	mutex_lock(&ptp->pincfg_mux);
> +	for (i = 0; i < ptp->info->n_pins; i++) {
> +		if (ptp->info->pin_config[i].func == func &&
> +		    ptp->info->pin_config[i].chan == chan) {
> +			pin = &ptp->info->pin_config[i];
> +			break;
> +		}
> +	}
> +	mutex_unlock(&ptp->pincfg_mux);
> +
> +	return pin ? pin - ptp->info->pin_config : -1;
> +}

Maybe replace 'pin - ptp->info->pin_config' with 'i'?

> +++ b/drivers/ptp/ptp_private.h
> @@ -48,6 +48,7 @@ struct ptp_clock {
>  	long dialed_frequency; /* remembers the frequency adjustment */
>  	struct timestamp_event_queue tsevq; /* simple fifo for time stamps */
>  	struct mutex tsevq_mux; /* one process at a time reading the fifo */
> +	struct mutex pincfg_mux; /* protect concurrent info->pin_config access */

Line over 80 chars. 

> + * @setpin:   Confirm that a pin can perform a given function. The PTP
> + *            Hardware Clock subsystem maintains the 'pin_config'
> + *            array on behalf of the drivers, but the PHC subsystem
> + *            assumes that every pin can perform every function. This
> + *            hook gives drivers a way of telling the core about
> + *            limitations on specific pins. This function must return
> + *            zero if the function can be assigned to this pin, and
> + *            nonzero otherwise.

The function does not actually set the pin. Rename to verify_pinfunc?

Stefan