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] [day] [month] [year] [list]
Message-ID: <20130424082209.GA25498@core.coreip.homeip.net>
Date:	Wed, 24 Apr 2013 01:22:10 -0700
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Chao Xie <chao.xie@...vell.com>
Cc:	linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
	haojian.zhuang@...il.com, xiechao.mail@...il.com
Subject: Re: [PATCH 1/6] input: pxa27x-keypad: copy members of platform data
 to device private data

Hi Chao,

On Tue, Apr 23, 2013 at 11:20:28PM -0400, Chao Xie wrote:
> Original driver will directly use platform data when driver is
> running.
> In fact, the platform data may be freed after system is bootup,

This statement is not correct, the platform data should be never be
freed, otherwise one can not reload a driver.

> or pointer for platform data is NULL if it has device tree support.
> Define the useful members of platform data in device private data.

Usually people just allocate platform data structure and fill it with DT
data.

Thanks.

> 
> Signed-off-by: Chao Xie <chao.xie@...vell.com>
> ---
>  drivers/input/keyboard/pxa27x_keypad.c |   97 ++++++++++++++++++++-----------
>  1 files changed, 62 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/input/keyboard/pxa27x_keypad.c b/drivers/input/keyboard/pxa27x_keypad.c
> index 5330d8f..023243a 100644
> --- a/drivers/input/keyboard/pxa27x_keypad.c
> +++ b/drivers/input/keyboard/pxa27x_keypad.c
> @@ -100,8 +100,6 @@
>  #define MAX_KEYPAD_KEYS		(MAX_MATRIX_KEY_NUM + MAX_DIRECT_KEY_NUM)
>  
>  struct pxa27x_keypad {
> -	struct pxa27x_keypad_platform_data *pdata;
> -
>  	struct clk *clk;
>  	struct input_dev *input_dev;
>  	void __iomem *mmio_base;
> @@ -116,15 +114,33 @@ struct pxa27x_keypad {
>  	uint32_t direct_key_state;
>  
>  	unsigned int direct_key_mask;
> +
> +	/* from platform data */
> +	unsigned int matrix_key_rows;
> +	unsigned int matrix_key_cols;
> +
> +	int direct_key_low_active;
> +	unsigned int direct_key_num;
> +	unsigned int default_direct_key_mask;
> +
> +	int enable_rotary0;
> +	int enable_rotary1;
> +
> +	unsigned int debounce_interval;
> +
> +	void (*clear_wakeup_event)(void);
>  };
>  
> -static void pxa27x_keypad_build_keycode(struct pxa27x_keypad *keypad)
> +static int pxa27x_keypad_build_keycode(struct device *dev,
> +			struct pxa27x_keypad *keypad,
> +			struct pxa27x_keypad_platform_data *pdata,
> +			struct input_dev *input_dev)
>  {
> -	struct pxa27x_keypad_platform_data *pdata = keypad->pdata;
> -	struct input_dev *input_dev = keypad->input_dev;
>  	unsigned short keycode;
>  	int i;
>  
> +	keypad->matrix_key_rows = pdata->matrix_key_rows;
> +	keypad->matrix_key_cols = pdata->matrix_key_cols;
>  	for (i = 0; i < pdata->matrix_key_map_size; i++) {
>  		unsigned int key = pdata->matrix_key_map[i];
>  		unsigned int row = KEY_ROW(key);
> @@ -137,12 +153,16 @@ static void pxa27x_keypad_build_keycode(struct pxa27x_keypad *keypad)
>  		__set_bit(keycode, input_dev->keybit);
>  	}
>  
> +	keypad->direct_key_low_active = pdata->direct_key_low_active;
> +	keypad->direct_key_num = pdata->direct_key_num;
> +	keypad->default_direct_key_mask = pdata->direct_key_mask;
>  	for (i = 0; i < pdata->direct_key_num; i++) {
>  		keycode = pdata->direct_key_map[i];
>  		keypad->keycodes[MAX_MATRIX_KEY_NUM + i] = keycode;
>  		__set_bit(keycode, input_dev->keybit);
>  	}
>  
> +	keypad->enable_rotary0 = pdata->enable_rotary0;
>  	if (pdata->enable_rotary0) {
>  		if (pdata->rotary0_up_key && pdata->rotary0_down_key) {
>  			keycode = pdata->rotary0_up_key;
> @@ -160,6 +180,7 @@ static void pxa27x_keypad_build_keycode(struct pxa27x_keypad *keypad)
>  		}
>  	}
>  
> +	keypad->enable_rotary1 = pdata->enable_rotary1;
>  	if (pdata->enable_rotary1) {
>  		if (pdata->rotary1_up_key && pdata->rotary1_down_key) {
>  			keycode = pdata->rotary1_up_key;
> @@ -178,11 +199,14 @@ static void pxa27x_keypad_build_keycode(struct pxa27x_keypad *keypad)
>  	}
>  
>  	__clear_bit(KEY_RESERVED, input_dev->keybit);
> +
> +	keypad->debounce_interval = pdata->debounce_interval;
> +
> +	return 0;
>  }
>  
>  static void pxa27x_keypad_scan_matrix(struct pxa27x_keypad *keypad)
>  {
> -	struct pxa27x_keypad_platform_data *pdata = keypad->pdata;
>  	struct input_dev *input_dev = keypad->input_dev;
>  	int row, col, num_keys_pressed = 0;
>  	uint32_t new_state[MAX_MATRIX_KEY_COLS];
> @@ -200,8 +224,8 @@ static void pxa27x_keypad_scan_matrix(struct pxa27x_keypad *keypad)
>  		row = KPAS_RP(kpas);
>  
>  		/* if invalid row/col, treat as no key pressed */
> -		if (col >= pdata->matrix_key_cols ||
> -		    row >= pdata->matrix_key_rows)
> +		if (col >= keypad->matrix_key_cols ||
> +		    row >= keypad->matrix_key_rows)
>  			goto scan;
>  
>  		new_state[col] = (1 << row);
> @@ -224,7 +248,7 @@ static void pxa27x_keypad_scan_matrix(struct pxa27x_keypad *keypad)
>  		new_state[7] = (kpasmkp3 >> 16) & KPASMKP_MKC_MASK;
>  	}
>  scan:
> -	for (col = 0; col < pdata->matrix_key_cols; col++) {
> +	for (col = 0; col < keypad->matrix_key_cols; col++) {
>  		uint32_t bits_changed;
>  		int code;
>  
> @@ -232,7 +256,7 @@ scan:
>  		if (bits_changed == 0)
>  			continue;
>  
> -		for (row = 0; row < pdata->matrix_key_rows; row++) {
> +		for (row = 0; row < keypad->matrix_key_rows; row++) {
>  			if ((bits_changed & (1 << row)) == 0)
>  				continue;
>  
> @@ -284,23 +308,21 @@ static void report_rotary_event(struct pxa27x_keypad *keypad, int r, int delta)
>  
>  static void pxa27x_keypad_scan_rotary(struct pxa27x_keypad *keypad)
>  {
> -	struct pxa27x_keypad_platform_data *pdata = keypad->pdata;
>  	uint32_t kprec;
>  
>  	/* read and reset to default count value */
>  	kprec = keypad_readl(KPREC);
>  	keypad_writel(KPREC, DEFAULT_KPREC);
>  
> -	if (pdata->enable_rotary0)
> +	if (keypad->enable_rotary0)
>  		report_rotary_event(keypad, 0, rotary_delta(kprec));
>  
> -	if (pdata->enable_rotary1)
> +	if (keypad->enable_rotary1)
>  		report_rotary_event(keypad, 1, rotary_delta(kprec >> 16));
>  }
>  
>  static void pxa27x_keypad_scan_direct(struct pxa27x_keypad *keypad)
>  {
> -	struct pxa27x_keypad_platform_data *pdata = keypad->pdata;
>  	struct input_dev *input_dev = keypad->input_dev;
>  	unsigned int new_state;
>  	uint32_t kpdk, bits_changed;
> @@ -308,14 +330,14 @@ static void pxa27x_keypad_scan_direct(struct pxa27x_keypad *keypad)
>  
>  	kpdk = keypad_readl(KPDK);
>  
> -	if (pdata->enable_rotary0 || pdata->enable_rotary1)
> +	if (keypad->enable_rotary0 || keypad->enable_rotary1)
>  		pxa27x_keypad_scan_rotary(keypad);
>  
>  	/*
>  	 * The KPDR_DK only output the key pin level, so it relates to board,
>  	 * and low level may be active.
>  	 */
> -	if (pdata->direct_key_low_active)
> +	if (keypad->direct_key_low_active)
>  		new_state = ~KPDK_DK(kpdk) & keypad->direct_key_mask;
>  	else
>  		new_state = KPDK_DK(kpdk) & keypad->direct_key_mask;
> @@ -325,7 +347,7 @@ static void pxa27x_keypad_scan_direct(struct pxa27x_keypad *keypad)
>  	if (bits_changed == 0)
>  		return;
>  
> -	for (i = 0; i < pdata->direct_key_num; i++) {
> +	for (i = 0; i < keypad->direct_key_num; i++) {
>  		if (bits_changed & (1 << i)) {
>  			int code = MAX_MATRIX_KEY_NUM + i;
>  
> @@ -340,10 +362,9 @@ static void pxa27x_keypad_scan_direct(struct pxa27x_keypad *keypad)
>  
>  static void clear_wakeup_event(struct pxa27x_keypad *keypad)
>  {
> -	struct pxa27x_keypad_platform_data *pdata = keypad->pdata;
>  
> -	if (pdata->clear_wakeup_event)
> -		(pdata->clear_wakeup_event)();
> +	if (keypad->clear_wakeup_event)
> +		(keypad->clear_wakeup_event)();
>  }
>  
>  static irqreturn_t pxa27x_keypad_irq_handler(int irq, void *dev_id)
> @@ -364,7 +385,6 @@ static irqreturn_t pxa27x_keypad_irq_handler(int irq, void *dev_id)
>  
>  static void pxa27x_keypad_config(struct pxa27x_keypad *keypad)
>  {
> -	struct pxa27x_keypad_platform_data *pdata = keypad->pdata;
>  	unsigned int mask = 0, direct_key_num = 0;
>  	unsigned long kpc = 0;
>  
> @@ -372,34 +392,34 @@ static void pxa27x_keypad_config(struct pxa27x_keypad *keypad)
>  	keypad_readl(KPC);
>  
>  	/* enable matrix keys with automatic scan */
> -	if (pdata->matrix_key_rows && pdata->matrix_key_cols) {
> +	if (keypad->matrix_key_rows && keypad->matrix_key_cols) {
>  		kpc |= KPC_ASACT | KPC_MIE | KPC_ME | KPC_MS_ALL;
> -		kpc |= KPC_MKRN(pdata->matrix_key_rows) |
> -		       KPC_MKCN(pdata->matrix_key_cols);
> +		kpc |= KPC_MKRN(keypad->matrix_key_rows) |
> +		       KPC_MKCN(keypad->matrix_key_cols);
>  	}
>  
>  	/* enable rotary key, debounce interval same as direct keys */
> -	if (pdata->enable_rotary0) {
> +	if (keypad->enable_rotary0) {
>  		mask |= 0x03;
>  		direct_key_num = 2;
>  		kpc |= KPC_REE0;
>  	}
>  
> -	if (pdata->enable_rotary1) {
> +	if (keypad->enable_rotary1) {
>  		mask |= 0x0c;
>  		direct_key_num = 4;
>  		kpc |= KPC_REE1;
>  	}
>  
> -	if (pdata->direct_key_num > direct_key_num)
> -		direct_key_num = pdata->direct_key_num;
> +	if (keypad->direct_key_num > direct_key_num)
> +		direct_key_num = keypad->direct_key_num;
>  
>  	/*
>  	 * Direct keys usage may not start from KP_DKIN0, check the platfrom
>  	 * mask data to config the specific.
>  	 */
> -	if (pdata->direct_key_mask)
> -		keypad->direct_key_mask = pdata->direct_key_mask;
> +	if (keypad->default_direct_key_mask)
> +		keypad->direct_key_mask = keypad->default_direct_key_mask;
>  	else
>  		keypad->direct_key_mask = ((1 << direct_key_num) - 1) & ~mask;
>  
> @@ -409,7 +429,7 @@ static void pxa27x_keypad_config(struct pxa27x_keypad *keypad)
>  
>  	keypad_writel(KPC, kpc | KPC_RE_ZERO_DEB);
>  	keypad_writel(KPREC, DEFAULT_KPREC);
> -	keypad_writel(KPKDI, pdata->debounce_interval);
> +	keypad_writel(KPKDI, keypad->debounce_interval);
>  }
>  
>  static int pxa27x_keypad_open(struct input_dev *dev)
> @@ -515,7 +535,6 @@ static int pxa27x_keypad_probe(struct platform_device *pdev)
>  		goto failed_free;
>  	}
>  
> -	keypad->pdata = pdata;
>  	keypad->input_dev = input_dev;
>  	keypad->irq = irq;
>  
> @@ -555,10 +574,18 @@ static int pxa27x_keypad_probe(struct platform_device *pdev)
>  	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
>  	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
>  
> -	pxa27x_keypad_build_keycode(keypad);
> +	if (pdata->clear_wakeup_event)
> +		keypad->clear_wakeup_event = pdata->clear_wakeup_event;
> +
> +	error = pxa27x_keypad_build_keycode(&pdev->dev, keypad, pdata,
> +					    input_dev);
> +	if (error) {
> +		dev_err(&pdev->dev, "failed to build keycode\n");
> +		goto failed_put_clk;
> +	}
>  
> -	if ((pdata->enable_rotary0 && keypad->rotary_rel_code[0] != -1) ||
> -	    (pdata->enable_rotary1 && keypad->rotary_rel_code[1] != -1)) {
> +	if ((keypad->enable_rotary0 && keypad->rotary_rel_code[0] != -1) ||
> +	    (keypad->enable_rotary1 && keypad->rotary_rel_code[1] != -1)) {
>  		input_dev->evbit[0] |= BIT_MASK(EV_REL);
>  	}
>  
> -- 
> 1.7.4.1
> 

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ