linux-kernel - Re: [PATCH 2/3] mfd: Add initial S5M8751 support

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]

Date:	Wed, 22 Jun 2011 13:48:20 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Sangbeom Kim <sbkim73@...sung.com>
Cc:	sameo@...ux.intel.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] mfd: Add initial S5M8751 support

On Wed, Jun 22, 2011 at 02:53:56PM +0900, Sangbeom Kim wrote:
> The WM8994 is a advanced PMIC with AUdio DAC
> Since it includes regulators, Battery charger, audio dac,
> it is represented as a multi-function device,
> though the functionality will be provided by the each driver.

I think you may have cut'n'pasted bits of this changelog from somewhere
else without doing all the updates you meant to :)

> +	if (event1 & S5M8751_MASK_PWRKEY1B)
> +		s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY1B);
> +
> +	if (event1 & S5M8751_MASK_PWRKEY2B)
> +		s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY2B);
> +
> +	if (event1 & S5M8751_MASK_PWRKEY3)
> +		s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY3);

This looks like the IRQ handler code doesn't need to understand the
specific events, it can just loop over the registers and fire off
interrupts based on the bit numbers.

As I said for the first patch this should all be using genirq.

> +static irqreturn_t s5m8751_irq(int irq, void *data)
> +{
> +	struct s5m8751 *s5m8751 = data;
> +
> +	disable_irq_nosync(irq);
> +	schedule_work(&s5m8751->irq_work);
> +
> +	return IRQ_HANDLED;
> +}

Use a threaded IRQ handler.

> +int s5m8751_device_init(struct s5m8751 *s5m8751, int irq,
> +			struct s5m8751_platform_data *pdata)
> +{
> +	int ret = -EINVAL;
> +	u8 chip_id;
> +
> +	if (pdata->init) {
> +		ret = pdata->init(s5m8751);
> +		if (ret != 0) {
> +			dev_err(s5m8751->dev,
> +			 "Platform init() failed: %d\n", ret);
> +			goto err;
> +		}
> +	}
> +
> +	s5m8751_reg_read(s5m8751, S5M8751_CHIP_ID, &chip_id);
> +	if (!chip_id)
> +		dev_info(s5m8751->dev, "Found S5M8751 device\n");
> +	else {
> +		dev_info(s5m8751->dev, "Didn't Find S5M8751 device\n");
> +		ret = -EINVAL;
> +		goto err;
> +	}

I'd really expect the init() callback to run after we've checked that
we can talk to the device.
--
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/