linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 1/9] w1_therm: creating w1_therm.h
@ 2020-04-29 22:59 Akira Shimahara
  2020-05-05 14:48 ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: Akira Shimahara @ 2020-04-29 22:59 UTC (permalink / raw)
  To: greg; +Cc: zbr, linux-kernel, Akira Shimahara

Creating w1_therm.h header to organize code. Organize the w1_therm.c file
to gather hardware functions, device specific functions, interface
functions and sysfs functions.

Signed-off-by: Akira Shimahara <akira215corp@gmail.com>
---
 drivers/w1/slaves/w1_therm.c | 302 +++++++++++++++--------------------
 drivers/w1/slaves/w1_therm.h | 138 ++++++++++++++++
 2 files changed, 269 insertions(+), 171 deletions(-)
 create mode 100644 drivers/w1/slaves/w1_therm.h

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 18f08d7..f027360 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -5,19 +5,15 @@
  * Copyright (c) 2004 Evgeniy Polyakov <zbr@ioremap.net>
  */
 
-#include <asm/types.h>
-
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/sched.h>
-#include <linux/device.h>
-#include <linux/types.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/hwmon.h>
 
-#include <linux/w1.h>
+#include "w1_therm.h"
 
 #define W1_THERM_DS18S20	0x10
 #define W1_THERM_DS1822		0x22
@@ -41,55 +37,6 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
-struct w1_therm_family_data {
-	uint8_t rom[9];
-	atomic_t refcnt;
-};
-
-struct therm_info {
-	u8 rom[9];
-	u8 crc;
-	u8 verdict;
-};
-
-/* return the address of the refcnt in the family data */
-#define THERM_REFCNT(family_data) \
-	(&((struct w1_therm_family_data *)family_data)->refcnt)
-
-static int w1_therm_add_slave(struct w1_slave *sl)
-{
-	sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
-		GFP_KERNEL);
-	if (!sl->family_data)
-		return -ENOMEM;
-	atomic_set(THERM_REFCNT(sl->family_data), 1);
-	return 0;
-}
-
-static void w1_therm_remove_slave(struct w1_slave *sl)
-{
-	int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data));
-
-	while (refcnt) {
-		msleep(1000);
-		refcnt = atomic_read(THERM_REFCNT(sl->family_data));
-	}
-	kfree(sl->family_data);
-	sl->family_data = NULL;
-}
-
-static ssize_t w1_slave_show(struct device *device,
-	struct device_attribute *attr, char *buf);
-
-static ssize_t w1_slave_store(struct device *device,
-	struct device_attribute *attr, const char *buf, size_t size);
-
-static ssize_t w1_seq_show(struct device *device,
-	struct device_attribute *attr, char *buf);
-
-static DEVICE_ATTR_RW(w1_slave);
-static DEVICE_ATTR_RO(w1_seq);
-
 static struct attribute *w1_therm_attrs[] = {
 	&dev_attr_w1_slave.attr,
 	NULL,
@@ -101,6 +48,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
 	NULL,
 };
 
+/*------------------------------attribute groups----------------------------*/
 ATTRIBUTE_GROUPS(w1_therm);
 ATTRIBUTE_GROUPS(w1_ds28ea00);
 
@@ -154,6 +102,7 @@ static const struct hwmon_chip_info w1_chip_info = {
 #define W1_CHIPINFO	NULL
 #endif
 
+/*------------------------------family operations---------------------------*/
 static struct w1_family_ops w1_therm_fops = {
 	.add_slave	= w1_therm_add_slave,
 	.remove_slave	= w1_therm_remove_slave,
@@ -168,6 +117,7 @@ static struct w1_family_ops w1_ds28ea00_fops = {
 	.chip_info	= W1_CHIPINFO,
 };
 
+/*--------------------family binding on operations struct-------------------*/
 static struct w1_family w1_therm_family_DS18S20 = {
 	.fid = W1_THERM_DS18S20,
 	.fops = &w1_therm_fops,
@@ -193,26 +143,7 @@ static struct w1_family w1_therm_family_DS1825 = {
 	.fops = &w1_therm_fops,
 };
 
-struct w1_therm_family_converter {
-	u8			broken;
-	u16			reserved;
-	struct w1_family	*f;
-	int			(*convert)(u8 rom[9]);
-	int			(*precision)(struct device *device, int val);
-	int			(*eeprom)(struct device *device);
-};
-
-/* write configuration to eeprom */
-static inline int w1_therm_eeprom(struct device *device);
-
-/* Set precision for conversion */
-static inline int w1_DS18B20_precision(struct device *device, int val);
-static inline int w1_DS18S20_precision(struct device *device, int val);
-
-/* The return value is millidegrees Centigrade. */
-static inline int w1_DS18B20_convert_temp(u8 rom[9]);
-static inline int w1_DS18S20_convert_temp(u8 rom[9]);
-
+/*----------------------Device capability description-----------------------*/
 static struct w1_therm_family_converter w1_therm_families[] = {
 	{
 		.f		= &w1_therm_family_DS18S20,
@@ -246,78 +177,7 @@ static struct w1_therm_family_converter w1_therm_families[] = {
 	}
 };
 
-static inline int w1_therm_eeprom(struct device *device)
-{
-	struct w1_slave *sl = dev_to_w1_slave(device);
-	struct w1_master *dev = sl->master;
-	u8 rom[9], external_power;
-	int ret, max_trying = 10;
-	u8 *family_data = sl->family_data;
-
-	if (!sl->family_data) {
-		ret = -ENODEV;
-		goto error;
-	}
-
-	/* prevent the slave from going away in sleep */
-	atomic_inc(THERM_REFCNT(family_data));
-
-	ret = mutex_lock_interruptible(&dev->bus_mutex);
-	if (ret != 0)
-		goto dec_refcnt;
-
-	memset(rom, 0, sizeof(rom));
-
-	while (max_trying--) {
-		if (!w1_reset_select_slave(sl)) {
-			unsigned int tm = 10;
-			unsigned long sleep_rem;
-
-			/* check if in parasite mode */
-			w1_write_8(dev, W1_READ_PSUPPLY);
-			external_power = w1_read_8(dev);
-
-			if (w1_reset_select_slave(sl))
-				continue;
-
-			/* 10ms strong pullup/delay after the copy command */
-			if (w1_strong_pullup == 2 ||
-			    (!external_power && w1_strong_pullup))
-				w1_next_pullup(dev, tm);
-
-			w1_write_8(dev, W1_COPY_SCRATCHPAD);
-
-			if (external_power) {
-				mutex_unlock(&dev->bus_mutex);
-
-				sleep_rem = msleep_interruptible(tm);
-				if (sleep_rem != 0) {
-					ret = -EINTR;
-					goto dec_refcnt;
-				}
-
-				ret = mutex_lock_interruptible(&dev->bus_mutex);
-				if (ret != 0)
-					goto dec_refcnt;
-			} else if (!w1_strong_pullup) {
-				sleep_rem = msleep_interruptible(tm);
-				if (sleep_rem != 0) {
-					ret = -EINTR;
-					goto mt_unlock;
-				}
-			}
-
-			break;
-		}
-	}
-
-mt_unlock:
-	mutex_unlock(&dev->bus_mutex);
-dec_refcnt:
-	atomic_dec(THERM_REFCNT(family_data));
-error:
-	return ret;
-}
+/*------------------------ Device dependent func---------------------------*/
 
 /* DS18S20 does not feature configuration register */
 static inline int w1_DS18S20_precision(struct device *device, int val)
@@ -434,40 +294,40 @@ static inline int w1_DS18S20_convert_temp(u8 rom[9])
 	return t;
 }
 
-static inline int w1_convert_temp(u8 rom[9], u8 fid)
+/*-------------------------Interface Functions------------------------------*/
+static int w1_therm_add_slave(struct w1_slave *sl)
 {
-	int i;
+	sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
+		GFP_KERNEL);
+	if (!sl->family_data)
+		return -ENOMEM;
+	atomic_set(THERM_REFCNT(sl->family_data), 1);
+	return 0;
+}
 
-	for (i = 0; i < ARRAY_SIZE(w1_therm_families); ++i)
-		if (w1_therm_families[i].f->fid == fid)
-			return w1_therm_families[i].convert(rom);
+static void w1_therm_remove_slave(struct w1_slave *sl)
+{
+	int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data));
 
-	return 0;
+	while (refcnt) {
+		msleep(1000);
+		refcnt = atomic_read(THERM_REFCNT(sl->family_data));
+	}
+	kfree(sl->family_data);
+	sl->family_data = NULL;
 }
 
-static ssize_t w1_slave_store(struct device *device,
-			      struct device_attribute *attr, const char *buf,
-			      size_t size)
+/*------------------------Hardware Functions--------------------------*/
+
+static inline int w1_convert_temp(u8 rom[9], u8 fid)
 {
-	int val, ret;
-	struct w1_slave *sl = dev_to_w1_slave(device);
 	int i;
 
-	ret = kstrtoint(buf, 0, &val);
-	if (ret)
-		return ret;
+	for (i = 0; i < ARRAY_SIZE(w1_therm_families); ++i)
+		if (w1_therm_families[i].f->fid == fid)
+			return w1_therm_families[i].convert(rom);
 
-	for (i = 0; i < ARRAY_SIZE(w1_therm_families); ++i) {
-		if (w1_therm_families[i].f->fid == sl->family->fid) {
-			/* zero value indicates to write current configuration to eeprom */
-			if (val == 0)
-				ret = w1_therm_families[i].eeprom(device);
-			else
-				ret = w1_therm_families[i].precision(device, val);
-			break;
-		}
-	}
-	return ret ? : size;
+	return 0;
 }
 
 static ssize_t read_therm(struct device *device,
@@ -564,6 +424,81 @@ error:
 	return ret;
 }
 
+static inline int w1_therm_eeprom(struct device *device)
+{
+	struct w1_slave *sl = dev_to_w1_slave(device);
+	struct w1_master *dev = sl->master;
+	u8 rom[9], external_power;
+	int ret, max_trying = 10;
+	u8 *family_data = sl->family_data;
+
+	if (!sl->family_data) {
+		ret = -ENODEV;
+		goto error;
+	}
+
+	/* prevent the slave from going away in sleep */
+	atomic_inc(THERM_REFCNT(family_data));
+
+	ret = mutex_lock_interruptible(&dev->bus_mutex);
+	if (ret != 0)
+		goto dec_refcnt;
+
+	memset(rom, 0, sizeof(rom));
+
+	while (max_trying--) {
+		if (!w1_reset_select_slave(sl)) {
+			unsigned int tm = 10;
+			unsigned long sleep_rem;
+
+			/* check if in parasite mode */
+			w1_write_8(dev, W1_READ_PSUPPLY);
+			external_power = w1_read_8(dev);
+
+			if (w1_reset_select_slave(sl))
+				continue;
+
+			/* 10ms strong pullup/delay after the copy command */
+			if (w1_strong_pullup == 2 ||
+			    (!external_power && w1_strong_pullup))
+				w1_next_pullup(dev, tm);
+
+			w1_write_8(dev, W1_COPY_SCRATCHPAD);
+
+			if (external_power) {
+				mutex_unlock(&dev->bus_mutex);
+
+				sleep_rem = msleep_interruptible(tm);
+				if (sleep_rem != 0) {
+					ret = -EINTR;
+					goto dec_refcnt;
+				}
+
+				ret = mutex_lock_interruptible(&dev->bus_mutex);
+				if (ret != 0)
+					goto dec_refcnt;
+			} else if (!w1_strong_pullup) {
+				sleep_rem = msleep_interruptible(tm);
+				if (sleep_rem != 0) {
+					ret = -EINTR;
+					goto mt_unlock;
+				}
+			}
+
+			break;
+		}
+	}
+
+mt_unlock:
+	mutex_unlock(&dev->bus_mutex);
+dec_refcnt:
+	atomic_dec(THERM_REFCNT(family_data));
+error:
+	return ret;
+}
+
+/*------------------------Interface sysfs--------------------------*/
+
 static ssize_t w1_slave_show(struct device *device,
 			     struct device_attribute *attr, char *buf)
 {
@@ -597,6 +532,31 @@ static ssize_t w1_slave_show(struct device *device,
 	return ret;
 }
 
+static ssize_t w1_slave_store(struct device *device,
+			      struct device_attribute *attr, const char *buf,
+			      size_t size)
+{
+	struct w1_slave *sl = dev_to_w1_slave(device);
+	int val, ret, i;
+
+	ret = kstrtoint(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(w1_therm_families); ++i) {
+		if (w1_therm_families[i].f->fid == sl->family->fid) {
+	/* zero value indicates to write current configuration to eeprom */
+			if (val == 0)
+				ret = w1_therm_families[i].eeprom(device);
+			else
+				ret = w1_therm_families[i].precision(device,
+								val);
+			break;
+		}
+	}
+	return ret ? : size;
+}
+
 #if IS_REACHABLE(CONFIG_HWMON)
 static int w1_read_temp(struct device *device, u32 attr, int channel,
 			long *val)
diff --git a/drivers/w1/slaves/w1_therm.h b/drivers/w1/slaves/w1_therm.h
new file mode 100644
index 0000000..8aa69cc
--- /dev/null
+++ b/drivers/w1/slaves/w1_therm.h
@@ -0,0 +1,138 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *	w1_therm.h
+ *
+ * Written by Akira Shimahara
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __W1_THERM_H
+#define __W1_THERM_H
+
+#include <asm/types.h>
+
+#include <linux/device.h>
+#include <linux/types.h>
+#include <linux/mutex.h>
+#include <linux/w1.h>
+
+/*----------------------------------Structs---------------------------------*/
+
+/**
+ * struct w1_therm_family_converter
+ * @brief Used to bind standard function call
+ * to device specific function
+ * it could be routed to NULL if device don't support feature
+ * see helper : device_family()
+ */
+struct w1_therm_family_converter {
+	u8			broken;
+	u16			reserved;
+	struct w1_family	*f;
+	int			(*convert)(u8 rom[9]);
+	int			(*precision)(struct device *device, int val);
+	int			(*eeprom)(struct device *device);
+};
+
+/**
+ * struct w1_therm_family_data
+ * @param rom data
+ * @param refcnt ref count
+ */
+struct w1_therm_family_data {
+	uint8_t rom[9];
+	atomic_t refcnt;
+};
+
+/**
+ * struct therm_info
+ * @brief Only used to store temperature reading
+ * @param rom RAM device data
+ * @param crc computed crc from rom
+ * @param verdict 1 crc checked, 0 crc not matching
+ */
+struct therm_info {
+	u8 rom[9];
+	u8 crc;
+	u8 verdict;
+};
+
+/*-----------------------Device specific functions-------------------------*/
+
+/* write configuration to eeprom */
+static inline int w1_therm_eeprom(struct device *device);
+
+/* Set precision for conversion */
+static inline int w1_DS18B20_precision(struct device *device, int val);
+static inline int w1_DS18S20_precision(struct device *device, int val);
+
+/* The return value is millidegrees Centigrade. */
+static inline int w1_DS18B20_convert_temp(u8 rom[9]);
+static inline int w1_DS18S20_convert_temp(u8 rom[9]);
+
+/*-------------------------------Macros--------------------------------------*/
+
+/* return the address of the refcnt in the family data */
+#define THERM_REFCNT(family_data) \
+	(&((struct w1_therm_family_data *)family_data)->refcnt)
+
+/*---------------------------Hardware Functions-----------------------------*/
+
+/** read_therm()
+ * @param sl pointer to the slave to read
+ * @param info pointer to a structure to store the read results
+ * @return 0 if success, -kernel error code otherwise
+ */
+static ssize_t read_therm(struct device *device,
+			struct w1_slave *sl, struct therm_info *info);
+
+/*----------------------------Interface sysfs-------------------------------*/
+
+/** @brief A callback function to output the temperature Old way
+ *  read temperature and return the result in the sys file
+ *  This has been kept for compatibility
+ */
+static ssize_t w1_slave_show(struct device *device,
+	struct device_attribute *attr, char *buf);
+
+/** @brief A callback function to set the resolution Old way
+ *  This has been kept for compatibility
+ *  @param 0, it write config in the EEPROM
+ *  @param 9..12, it set the resolution in the RAM
+ */
+static ssize_t w1_slave_store(struct device *device,
+	struct device_attribute *attr, const char *buf, size_t size);
+
+static ssize_t w1_seq_show(struct device *device,
+	struct device_attribute *attr, char *buf);
+
+/*-----------------------------Attributes declarations----------------------*/
+
+static DEVICE_ATTR_RW(w1_slave);
+static DEVICE_ATTR_RO(w1_seq);
+
+/*--------------------------Interface Functions-----------------------------*/
+
+/** w1_therm_add_slave() - Called each time a search discover a new device
+ * @brief used to initialized slave (family datas)
+ * @param sl slave just discovered
+ * @return 0 - If success, negative kernel code otherwise
+ */
+static int w1_therm_add_slave(struct w1_slave *sl);
+
+/** w1_therm_remove_slave() - Called each time a slave is removed
+ * @brief used to free memory
+ * @param sl slave to be removed
+ */
+static void w1_therm_remove_slave(struct w1_slave *sl);
+
+#endif /* __W1_THERM_H */
\ No newline at end of file
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/9] w1_therm: creating w1_therm.h
  2020-04-29 22:59 [PATCH v4 1/9] w1_therm: creating w1_therm.h Akira Shimahara
@ 2020-05-05 14:48 ` Greg KH
  2020-05-05 21:04   ` Akira shimahara
  0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2020-05-05 14:48 UTC (permalink / raw)
  To: Akira Shimahara; +Cc: zbr, linux-kernel

On Thu, Apr 30, 2020 at 12:59:15AM +0200, Akira Shimahara wrote:
> Creating w1_therm.h header to organize code. Organize the w1_therm.c file
> to gather hardware functions, device specific functions, interface
> functions and sysfs functions.
> 
> Signed-off-by: Akira Shimahara <akira215corp@gmail.com>
> ---
>  drivers/w1/slaves/w1_therm.c | 302 +++++++++++++++--------------------
>  drivers/w1/slaves/w1_therm.h | 138 ++++++++++++++++
>  2 files changed, 269 insertions(+), 171 deletions(-)
>  create mode 100644 drivers/w1/slaves/w1_therm.h

Wait, why is a .h file needed for just a single .c file?


<snip>

>  static ssize_t read_therm(struct device *device,


> +/** read_therm()
> + * @param sl pointer to the slave to read
> + * @param info pointer to a structure to store the read results
> + * @return 0 if success, -kernel error code otherwise
> + */
> +static ssize_t read_therm(struct device *device,
> +			struct w1_slave *sl, struct therm_info *info);
> +

Why is this function needed to be declared in this .h file?

Why is any of this needed?  For some reason I thought you needed a .h
file to make things simpler for other .c files, but if all of this is
static, it's not needed at all, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/9] w1_therm: creating w1_therm.h
  2020-05-05 14:48 ` Greg KH
@ 2020-05-05 21:04   ` Akira shimahara
  2020-05-06  6:51     ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: Akira shimahara @ 2020-05-05 21:04 UTC (permalink / raw)
  To: Greg KH; +Cc: zbr, linux-kernel

Le mardi 05 mai 2020 à 16:48 +0200, Greg KH a écrit :
> > Creating w1_therm.h header to organize code. Organize the
> > w1_therm.c file
> > to gather hardware functions, device specific functions, interface
> > functions and sysfs functions.
> > Signed-off-by: Akira Shimahara <akira215corp@gmail.com>
> > ---
> >   drivers/w1/slaves/w1_therm.c | 302 +++++++++++++++---------------
> > -----
> >   drivers/w1/slaves/w1_therm.h | 138 ++++++++++++++++
> >   2 files changed, 269 insertions(+), 171 deletions(-)
> >   create mode 100644 drivers/w1/slaves/w1_therm.h
> 
> 
> Wait, why is a .h file needed for just a single .c file?
> 
> 
> 
> 
> 
> <snip>
> 
> 
> 
> >   static ssize_t read_therm(struct device *device,
> 
> 
> 
> 
> > +/** read_therm()
> > + * @param sl pointer to the slave to read
> > + * @param info pointer to a structure to store the read results
> > + * @return 0 if success, -kernel error code otherwise
> > + */
> > +static ssize_t read_therm(struct device *device,
> > +                     struct w1_slave *sl, struct therm_info
> > *info);
> > +
> 
> 
> Why is this function needed to be declared in this .h file?
> 
> 
> 
> Why is any of this needed?  For some reason I thought you needed a .h
> 
> file to make things simpler for other .c files, but if all of this is
> 
> static, it's not needed at all, right?
> 
> 
> 
> thanks,
> 
> 
> 
> greg k-h

Hello,

Yes, you are right, header file could be avoided. But we separate it
from .c for clarity purpose, and to ease future developpment (for
example adding support of new devices).

If you absolutely want to put everything in the .c file, I can do it,
let me know.

Thanks ahead,

Akira Shimahara


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/9] w1_therm: creating w1_therm.h
  2020-05-05 21:04   ` Akira shimahara
@ 2020-05-06  6:51     ` Greg KH
  2020-05-06 10:43       ` Akira shimahara
  0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2020-05-06  6:51 UTC (permalink / raw)
  To: Akira shimahara; +Cc: zbr, linux-kernel

On Tue, May 05, 2020 at 11:04:39PM +0200, Akira shimahara wrote:
> Le mardi 05 mai 2020 à 16:48 +0200, Greg KH a écrit :
> > > Creating w1_therm.h header to organize code. Organize the
> > > w1_therm.c file
> > > to gather hardware functions, device specific functions, interface
> > > functions and sysfs functions.
> > > Signed-off-by: Akira Shimahara <akira215corp@gmail.com>
> > > ---
> > >   drivers/w1/slaves/w1_therm.c | 302 +++++++++++++++---------------
> > > -----
> > >   drivers/w1/slaves/w1_therm.h | 138 ++++++++++++++++
> > >   2 files changed, 269 insertions(+), 171 deletions(-)
> > >   create mode 100644 drivers/w1/slaves/w1_therm.h
> > 
> > 
> > Wait, why is a .h file needed for just a single .c file?
> > 
> > 
> > 
> > 
> > 
> > <snip>
> > 
> > 
> > 
> > >   static ssize_t read_therm(struct device *device,
> > 
> > 
> > 
> > 
> > > +/** read_therm()
> > > + * @param sl pointer to the slave to read
> > > + * @param info pointer to a structure to store the read results
> > > + * @return 0 if success, -kernel error code otherwise
> > > + */
> > > +static ssize_t read_therm(struct device *device,
> > > +                     struct w1_slave *sl, struct therm_info
> > > *info);
> > > +
> > 
> > 
> > Why is this function needed to be declared in this .h file?
> > 
> > 
> > 
> > Why is any of this needed?  For some reason I thought you needed a .h
> > 
> > file to make things simpler for other .c files, but if all of this is
> > 
> > static, it's not needed at all, right?
> > 
> > 
> > 
> > thanks,
> > 
> > 
> > 
> > greg k-h
> 
> Hello,
> 
> Yes, you are right, header file could be avoided. But we separate it
> from .c for clarity purpose, and to ease future developpment (for
> example adding support of new devices).
> 
> If you absolutely want to put everything in the .c file, I can do it,
> let me know.

Keep it all in a .c file, only use .h files for when you need to share
it across multiple .c files, otherwise it's not needed.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/9] w1_therm: creating w1_therm.h
  2020-05-06  6:51     ` Greg KH
@ 2020-05-06 10:43       ` Akira shimahara
  0 siblings, 0 replies; 5+ messages in thread
From: Akira shimahara @ 2020-05-06 10:43 UTC (permalink / raw)
  To: Greg KH; +Cc: zbr, linux-kernel

Le mercredi 06 mai 2020 à 08:51 +0200, Greg KH a écrit :
> > > Why is this function needed to be declared in this .h file?
> > > Why is any of this needed?  For some reason I thought you needed
> > > a .h
> > > file to make things simpler for other .c files, but if all of
> > > this is
> > > static, it's not needed at all, right?
> > > thanks,
> > > greg k-h
> > Hello,
> > Yes, you are right, header file could be avoided. But we separate
> > it
> > from .c for clarity purpose, and to ease future developpment (for
> > example adding support of new devices).
> > If you absolutely want to put everything in the .c file, I can do
> > it,
> > let me know.
> 
> 
> Keep it all in a .c file, only use .h files for when you need to
> share
> 
> it across multiple .c files, otherwise it's not needed.
> 
> 
> 
> thanks,
> 
> 
> 
> greg k-h

Hi, 

Ok well noted, I will do it tomorrow, and keep it as 8 patches.

Thanks for your time

Akira Shimahara


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-05-06 10:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29 22:59 [PATCH v4 1/9] w1_therm: creating w1_therm.h Akira Shimahara
2020-05-05 14:48 ` Greg KH
2020-05-05 21:04   ` Akira shimahara
2020-05-06  6:51     ` Greg KH
2020-05-06 10:43       ` Akira shimahara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).