Re: [PATCH] staging: tidspbridge: remove file handling functions for loader
On Tue, Dec 07, 2010 at 12:09:06AM -0600, Omar Ramirez Luna wrote: Instead use request_firmware and friends to get a valid firmware image. Right now the image is supplied dynamically through udev and the following rule: KERNEL==omap-dsp, SUBSYSTEM==firmware, ACTION==add, \ RUN+=/bin/sh -c 'echo 1 /sys/$DEVPATH/loading; \ cat $FIRMWARE /sys/$DEVPATH/data; \ echo 0 /sys/$DEVPATH/loading' Why do you need a custom firmware rule? Why doesn't the default firmware loading rule that comes with udev work properly for you? What are you needing different here that works properly for all other drivers? confused, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: remove file handling functions for loader
Hi, On Wed, Dec 8, 2010 at 4:26 PM, Greg KH g...@kroah.com wrote: On Tue, Dec 07, 2010 at 12:09:06AM -0600, Omar Ramirez Luna wrote: Instead use request_firmware and friends to get a valid firmware image. Right now the image is supplied dynamically through udev and the following rule: KERNEL==omap-dsp, SUBSYSTEM==firmware, ACTION==add, \ RUN+=/bin/sh -c 'echo 1 /sys/$DEVPATH/loading; \ cat $FIRMWARE /sys/$DEVPATH/data; \ echo 0 /sys/$DEVPATH/loading' Why do you need a custom firmware rule? It was meant as an example, when I compiled my minimal file system it didn't supply the firmware.sh script nor created /lib/firmware... I thought that not everybody would have the firmware.sh, so I just provided a sample rule. Why doesn't the default firmware loading rule that comes with udev work properly for you? What are you needing different here that works properly for all other drivers? firmware.sh under /lib/udev/ and dsp binaries installed under /lib/firmware/, my rule is the brute version of firmware.sh so nothing different in the script. Probably the only change would be to supply the firmware name only, as of now the insmod parameter requires the entire path, e.g.: insmod bridgedriver.ko base_img=/lib/dsp/baseimage.dof if using firmware.sh and placing firmware files under /lib/firmware/, then insmod bridgedriver.ko base_img=baseimage.dof Should be enough. Regards, Omar -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: remove file handling functions for loader
On Wed, Dec 08, 2010 at 05:02:20PM -0600, Ramirez Luna, Omar wrote: Hi, On Wed, Dec 8, 2010 at 4:26 PM, Greg KH g...@kroah.com wrote: On Tue, Dec 07, 2010 at 12:09:06AM -0600, Omar Ramirez Luna wrote: Instead use request_firmware and friends to get a valid firmware image. Right now the image is supplied dynamically through udev and the following rule: KERNEL==omap-dsp, SUBSYSTEM==firmware, ACTION==add, \ RUN+=/bin/sh -c 'echo 1 /sys/$DEVPATH/loading; \ cat $FIRMWARE /sys/$DEVPATH/data; \ echo 0 /sys/$DEVPATH/loading' Why do you need a custom firmware rule? It was meant as an example, when I compiled my minimal file system it didn't supply the firmware.sh script nor created /lib/firmware... I thought that not everybody would have the firmware.sh, so I just provided a sample rule. So, can I remove this from the changelog comment, as it's not really needed at all? Why doesn't the default firmware loading rule that comes with udev work properly for you? What are you needing different here that works properly for all other drivers? firmware.sh under /lib/udev/ and dsp binaries installed under /lib/firmware/, my rule is the brute version of firmware.sh so nothing different in the script. Probably the only change would be to supply the firmware name only, as of now the insmod parameter requires the entire path, e.g.: insmod bridgedriver.ko base_img=/lib/dsp/baseimage.dof if using firmware.sh and placing firmware files under /lib/firmware/, then insmod bridgedriver.ko base_img=baseimage.dof Ick, why use a module parameter name at all? Why is this special and different from all other firmware users? They don't have to manually specify a file name, the driver does that. Please fix up the patch to not require a module parameter, distros hate them, and users hate them even more. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: remove file handling functions for loader
On Wed, Dec 8, 2010 at 5:08 PM, Greg KH g...@kroah.com wrote: On Wed, Dec 08, 2010 at 05:02:20PM -0600, Ramirez Luna, Omar wrote: Hi, On Wed, Dec 8, 2010 at 4:26 PM, Greg KH g...@kroah.com wrote: On Tue, Dec 07, 2010 at 12:09:06AM -0600, Omar Ramirez Luna wrote: Instead use request_firmware and friends to get a valid firmware image. Right now the image is supplied dynamically through udev and the following rule: KERNEL==omap-dsp, SUBSYSTEM==firmware, ACTION==add, \ RUN+=/bin/sh -c 'echo 1 /sys/$DEVPATH/loading; \ cat $FIRMWARE /sys/$DEVPATH/data; \ echo 0 /sys/$DEVPATH/loading' Why do you need a custom firmware rule? It was meant as an example, when I compiled my minimal file system it didn't supply the firmware.sh script nor created /lib/firmware... I thought that not everybody would have the firmware.sh, so I just provided a sample rule. So, can I remove this from the changelog comment, as it's not really needed at all? Yes it can be removed. BTW, I don't expect this pushed through staging yet, I need to include it to my branch first and then I'll send a pull request with the pile of patches. Sorry for the misunderstanding and thanks for the review. insmod bridgedriver.ko base_img=baseimage.dof Ick, why use a module parameter name at all? Why is this special and different from all other firmware users? They don't have to manually specify a file name, the driver does that. The thing is that there are N number of firmwares, e.g.: There is the official and usable firmware to play with multimedia baseimage.dof But there are also minimal firmwares to just ping or swap buffers with the dsp, when you just want to play around with basic features. Please fix up the patch to not require a module parameter, distros hate them, and users hate them even more. The driver is the one requiring the parameter (it was already this way), this patch doesn't introduce any parameter. I'll check what can be done and if nobody rejects I'll use the baseimage.dof as firmware by default. Regards, Omar -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: remove file handling functions for loader
On Wed, Dec 08, 2010 at 05:32:50PM -0600, Ramirez Luna, Omar wrote: On Wed, Dec 8, 2010 at 5:08 PM, Greg KH g...@kroah.com wrote: On Wed, Dec 08, 2010 at 05:02:20PM -0600, Ramirez Luna, Omar wrote: Hi, On Wed, Dec 8, 2010 at 4:26 PM, Greg KH g...@kroah.com wrote: On Tue, Dec 07, 2010 at 12:09:06AM -0600, Omar Ramirez Luna wrote: Instead use request_firmware and friends to get a valid firmware image. Right now the image is supplied dynamically through udev and the following rule: KERNEL==omap-dsp, SUBSYSTEM==firmware, ACTION==add, \ RUN+=/bin/sh -c 'echo 1 /sys/$DEVPATH/loading; \ cat $FIRMWARE /sys/$DEVPATH/data; \ echo 0 /sys/$DEVPATH/loading' Why do you need a custom firmware rule? It was meant as an example, when I compiled my minimal file system it didn't supply the firmware.sh script nor created /lib/firmware... I thought that not everybody would have the firmware.sh, so I just provided a sample rule. So, can I remove this from the changelog comment, as it's not really needed at all? Yes it can be removed. BTW, I don't expect this pushed through staging yet, I need to include it to my branch first and then I'll send a pull request with the pile of patches. Sorry for the misunderstanding and thanks for the review. Well, don't send me patches you don't want me to apply without a big DO NOT APPLY in them, otherwise I might try to :) insmod bridgedriver.ko base_img=baseimage.dof Ick, why use a module parameter name at all? Why is this special and different from all other firmware users? They don't have to manually specify a file name, the driver does that. The thing is that there are N number of firmwares, e.g.: There is the official and usable firmware to play with multimedia baseimage.dof But there are also minimal firmwares to just ping or swap buffers with the dsp, when you just want to play around with basic features. Then mess with the firmware symlink in userspace, don't have the driver have to worry about it. Please fix up the patch to not require a module parameter, distros hate them, and users hate them even more. The driver is the one requiring the parameter (it was already this way), this patch doesn't introduce any parameter. I'll check what can be done and if nobody rejects I'll use the baseimage.dof as firmware by default. That would be good. Again, you can put whatever firmware image you want in that location if you wish to use different ones. That is for userspace to do, not have the kernel worry about. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] staging: tidspbridge: remove file handling functions for loader
Instead use request_firmware and friends to get a valid firmware
image.
Right now the image is supplied dynamically through udev and the
following rule:
KERNEL==omap-dsp, SUBSYSTEM==firmware, ACTION==add, \
RUN+=/bin/sh -c 'echo 1 /sys/$DEVPATH/loading; \
cat $FIRMWARE /sys/$DEVPATH/data; \
echo 0 /sys/$DEVPATH/loading'
Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com
---
.../tidspbridge/include/dspbridge/dbldefs.h| 10 --
.../staging/tidspbridge/include/dspbridge/dbll.h |7 ++
.../tidspbridge/include/dspbridge/dblldefs.h | 35 --
drivers/staging/tidspbridge/pmgr/cod.c | 100 -
drivers/staging/tidspbridge/pmgr/dbll.c| 114 +++
5 files changed, 73 insertions(+), 193 deletions(-)
diff --git a/drivers/staging/tidspbridge/include/dspbridge/dbldefs.h
b/drivers/staging/tidspbridge/include/dspbridge/dbldefs.h
index bf4fb99..c74321b 100644
--- a/drivers/staging/tidspbridge/include/dspbridge/dbldefs.h
+++ b/drivers/staging/tidspbridge/include/dspbridge/dbldefs.h
@@ -126,16 +126,6 @@ struct dbl_attrs {
dbl_sym_lookup sym_lookup;
void *sym_handle;
void *sym_arg;
-
- /*
-* These file manipulation functions should be compatible with the
-* C run time library functions of the same name.
-*/
-s32(*fread) (void *, size_t, size_t, void *);
-s32(*fseek) (void *, long, int);
-s32(*ftell) (void *);
-s32(*fclose) (void *);
- void *(*fopen) (const char *, const char *);
};
#endif /* DBLDEFS_ */
diff --git a/drivers/staging/tidspbridge/include/dspbridge/dbll.h
b/drivers/staging/tidspbridge/include/dspbridge/dbll.h
index b018676..ad081e0 100644
--- a/drivers/staging/tidspbridge/include/dspbridge/dbll.h
+++ b/drivers/staging/tidspbridge/include/dspbridge/dbll.h
@@ -20,9 +20,16 @@
#ifndef DBLL_
#define DBLL_
+#include linux/firmware.h
#include dspbridge/dbdefs.h
#include dspbridge/dblldefs.h
+struct dsp_fw {
+ const struct firmware *img;
+ const char *name;
+ const u8 *pos;
+};
+
extern bool symbols_reloaded;
extern void dbll_close(struct dbll_library_obj *zl_lib);
diff --git a/drivers/staging/tidspbridge/include/dspbridge/dblldefs.h
b/drivers/staging/tidspbridge/include/dspbridge/dblldefs.h
index d2b4fda..f353a14 100644
--- a/drivers/staging/tidspbridge/include/dspbridge/dblldefs.h
+++ b/drivers/staging/tidspbridge/include/dspbridge/dblldefs.h
@@ -79,11 +79,6 @@ typedef s32(*dbll_alloc_fxn) (void *hdl, s32 space, u32
size, u32 align,
bool reserved);
/*
- * dbll_close_fxn
- */
-typedef s32(*dbll_f_close_fxn) (void *);
-
-/*
* dbll_free_fxn
* Free memory function. Free, or unreserve (if reserved == TRUE) size
* bytes of memory from segment space
@@ -92,11 +87,6 @@ typedef bool(*dbll_free_fxn) (void *hdl, u32 addr, s32
space, u32 size,
bool reserved);
/*
- * dbll_f_open_fxn
- */
-typedef void *(*dbll_f_open_fxn) (const char *, const char *);
-
-/*
* dbll_log_write_fxn
* Function to call when writing data from a section, to log the info.
* Can be NULL if no logging is required.
@@ -106,16 +96,6 @@ typedef int(*dbll_log_write_fxn) (void *handle,
u32 bytes);
/*
- * dbll_read_fxn
- */
-typedef s32(*dbll_read_fxn) (void *, size_t, size_t, void *);
-
-/*
- * dbll_seek_fxn
- */
-typedef s32(*dbll_seek_fxn) (void *, long, int);
-
-/*
* dbll_sym_lookup
* Symbol lookup function - Find the symbol name and return its value.
*
@@ -133,11 +113,6 @@ typedef bool(*dbll_sym_lookup) (void *handle, void *parg,
void *rmm_handle,
const char *name, struct dbll_sym_val ** sym);
/*
- * dbll_tell_fxn
- */
-typedef s32(*dbll_tell_fxn) (void *);
-
-/*
* dbll_write_fxn
* Write memory function. Write n HOST bytes of memory to segment mtype
* starting at address dsp_address from the buffer buf. The buffer is
@@ -163,16 +138,6 @@ struct dbll_attrs {
dbll_sym_lookup sym_lookup;
void *sym_handle;
void *sym_arg;
-
- /*
-* These file manipulation functions should be compatible with the
-* C run time library functions of the same name.
-*/
-s32(*fread) (void *, size_t, size_t, void *);
-s32(*fseek) (void *, long, int);
-s32(*ftell) (void *);
-s32(*fclose) (void *);
- void *(*fopen) (const char *, const char *);
};
/*
diff --git a/drivers/staging/tidspbridge/pmgr/cod.c
b/drivers/staging/tidspbridge/pmgr/cod.c
index 52989ab..31cfa9b 100644
--- a/drivers/staging/tidspbridge/pmgr/cod.c
+++ b/drivers/staging/tidspbridge/pmgr/cod.c
@@ -89,101
