Thread (18 messages) 18 messages, 4 authors, 2016-01-26

[PATCH V2 4/4] mfd: mediatek: add MT6323 support to MT6397 driver

From: John Crispin <hidden>
Date: 2016-01-26 12:05:38
Also in: linux-mediatek, lkml


On 26/01/2016 09:34, Lee Jones wrote:
On Tue, 26 Jan 2016, John Crispin wrote:
quoted
On 26/01/2016 04:07, Henry Chen wrote:
quoted
On Mon, 2016-01-25 at 19:59 +0100, John Crispin wrote:
quoted
On 25/01/2016 19:44, Matthias Brugger wrote:
quoted
On Monday 25 Jan 2016 16:36:40 John Crispin wrote:
quoted
Hi,

On 25/01/2016 13:41, Lee Jones wrote:
quoted
Please honour the subject format of the subsystem you are contributing
to.

`git log --oneline -- $subsystem` gives you this.

On Mon, 25 Jan 2016, John Crispin wrote:
quoted
Signed-off-by: John Crispin <redacted>
---
[...]
quoted
quoted
@@ -261,6 +271,15 @@ static int mt6397_probe(struct platform_device
*pdev)

 	}
 	
 	switch (id & 0xff) {

+	case MT6323_CID_CODE:
+		mt6397->int_con[0] = MT6323_INT_CON0;
This is confusing.  You're still using memory allocated for a mt6397
device.
the variable is currently defined as struct mt6397_chip *mt6397;
shall i only change the name or also create a patch to rename the struct ?
I think we should rename the struct and the file as well.

Cheers,
Matthias
Hi,

that would have been my next question. renaming the struct would imply
renaming the driver and the whole namespace contained within. We would
then also need to change the Kconfig and Makefile. I am happy to do this
but want to be sure that is is actually wanted.

	John
Hi,

Since mt6323 was similar with mt6397, I think we can reuse the
mt6397_chip without duplicate code.

Maybe we can rename the local variable name to avoid confusing.

struct mt6397_chip *mt_pmic;
...
...
switch (id & 0xff) {
	case MT6323_CID_CODE:
		mt_pmic->int_con[0] = MT6323_INT_CON0;
		mt_pmic->int_con[1] = MT6323_INT_CON1;
...
...

Henry
Hi,

IMHO we should either rename the namespace or not. renaming some
variables seems weird as that will just move the confusion/inconsistency
to another place in the code. I am however rather indifferent on this
matter.
It's common to name a driver after the device which was enabled first,
so no need to rename the files or CONFIGs; however, it does seem
prudent to generify the struct (both parts).
Hi,

renaming struct mt6397_chip is not as straightforward as it seems as
other drivers such as pinctrl and rtc share the data structure with the
mfd driver.

i have kept the structs name as is for now and only renamed the instance.

If you want i can create a separate series that addresses only the
renaming of the structure. The "rename struct mt6397" patch would need
to do a change to drivers from 3 subsystems in 1 single commit as not to
break compile and merge order.

	John
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help